Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support and tests for Python 2.7 #234

Merged
merged 8 commits into from Nov 2, 2021
Merged

Remove support and tests for Python 2.7 #234

merged 8 commits into from Nov 2, 2021

Conversation

jklukas
Copy link
Member

@jklukas jklukas commented Oct 7, 2021

The next build will be 3.4+.

Todos

  • MIT compatible
  • Tests
  • Documentation
  • Updated CHANGES.rst

@jklukas jklukas requested a review from graingert October 7, 2021 23:06
.travis.yml Outdated
- "3.6"
- "3.7"
- "3.8"
- "3.4"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests on 3.6-3.8?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we've ever found an issue that's isolated to one of these intermediate versions. Given that tests are taking longer, it seems wasteful to consume resources on running tests across 5 different python versions. It seems sufficient to me to test on the oldest supported version and the newest version. What do you think?

@jklukas
Copy link
Member Author

jklukas commented Oct 8, 2021

I started messing with the tox configuration here and will need to do some more playing with it to get it working.

Copy link
Contributor

@Brooke-white Brooke-white left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this change, would we still need compat.py?

@jklukas
Copy link
Member Author

jklukas commented Nov 1, 2021

Given this change, would we still need compat.py?

Good catch. Will remove.

Comment on lines +16 to +17
- python: "3.9"
env: TOXENV=docs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A docs task was defined in tox.ini, but wasn't defined in the travis config. This allows it to run regularly in CI.

@@ -16,8 +16,9 @@ The package is available on PyPI::

.. warning::

This dialect requires psycopg2 library to work properly. It does not provide
it as required, but relies on you to select the psycopg2 distribution you need:
This dialect requires either ``redshift_connector`` or ``psycopg2``
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated docs fixup. I noticed this while restoring the docs task.

@@ -10,7 +10,6 @@ Contents:
ddl-compiler
dialect
commands
ddl
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an error in the index; there's no rst file of this name.

@@ -21,7 +21,7 @@ deps =
pytest==3.10.1
requests==2.7.0

[testenv:py{36,37,38,39}-sa{13,14}]
[testenv:py39-sa14]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reduce test runtime, I've changed the py39 test to only include sa14, and the 35 test includes only sa13. Tests can complete now in under 15 minutes.

@@ -45,7 +45,7 @@ changedir=docs
deps=
-rrequirements-docs.txt
commands=
sphinx-build -W -b html -d {envtmpdir}/doctrees . {envtmpdir}/html
sphinx-build -b html -d {envtmpdir}/doctrees . {envtmpdir}/html
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the -W flag because docs generation currently fails due to warnings. I filed #239 to follow up on addressing the warnings.

@jklukas jklukas merged commit c830f7b into main Nov 2, 2021
@jklukas jklukas deleted the remove-2.7 branch November 2, 2021 14:39
@jklukas jklukas removed the request for review from graingert November 2, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants