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

Commit database after creating oauth_tokens table. Fixes #75 #76

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@hunterji
Copy link
Contributor

hunterji commented Jan 21, 2017

This change seems to be required in Python 3.6.0. It works correctly without the db.commit() in 3.5.2 and earlier. Without the commit the first table, oauth_cache_db_version, gets created, but oauth_tokens does not.

I ran the unit tests on 3.5.2 after making the change and all passed. I couldn't run them on 3.6.0 because pytest isn't supported there yet. I did make the change and test manually in 3.6.0. All tests were on Windows 10.

Commit database after creating oauth_tokens table. Fixes #75
This change seems to be required in Python 3.6.0. It works correctly without the db.commit() in 3.5.2 and earlier. Without the commit the first table, oauth_cache_db_version, gets created, but oauth_tokens does not.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 21, 2017

Coverage Status

Coverage increased (+0.04%) to 67.571% when pulling e6c8004 on hunterji:master into 142d199 on sybrenstuvel:master.

@thijstriemstra

This comment has been minimized.

Copy link
Collaborator

thijstriemstra commented Jan 26, 2017

@hunterji can you also add python 3.6 to tox/travis?

@hunterji

This comment has been minimized.

Copy link
Contributor Author

hunterji commented Jan 27, 2017

I added py36 to tox.ini

@hunterji

This comment has been minimized.

Copy link
Contributor Author

hunterji commented Jan 27, 2017

Adding py36 causes TravisCI to fail the checks. Is this because pytest doesn't support Python 3.6 yet? That is why I hadn't added py36 to tox.ini, but I've never used TravisCI so I'm just guessing.

@sybrenstuvel

This comment has been minimized.

Copy link
Owner

sybrenstuvel commented Jan 28, 2017

To get TravisCI to play along, you also need to change .travis.yml; it's a good idea to edit that one too. However, it is not the cause of the error that TravisCI is showing:

VersionConflict: (six 1.9.0 (/home/travis/build/sybrenstuvel/flickrapi/.tox/pypy/site-packages), Requirement.parse('six>=1.10.0'))

I'm not sure what exactly requires six>=1.10.0, maybe that's the first version that supports Python 3.6?

@sybrenstuvel

This comment has been minimized.

Copy link
Owner

sybrenstuvel commented Feb 6, 2017

Please do only one thing per pull request. That makes it much easier to review & accept.

@sybrenstuvel

This comment has been minimized.

Copy link
Owner

sybrenstuvel commented Feb 6, 2017

Cherry-picked original PR in 0556a7e.

@sybrenstuvel

This comment has been minimized.

Copy link
Owner

sybrenstuvel commented Feb 6, 2017

I've added Python 3.6 to Tox & Travis in 4e256ab, rewriting how we set up Travis in the first place.

Before this commit, Travis multiplied the Python versions with the environments, and ran the tests for each combination. The new approach sets things up correctly, matching Python version with TOXENV variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment