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

Fix Python 2.6 on Appveyor CI #53

Merged
merged 3 commits into from
Oct 12, 2017
Merged

Fix Python 2.6 on Appveyor CI #53

merged 3 commits into from
Oct 12, 2017

Conversation

shakeelmohamed
Copy link
Contributor

Add version ceiling on cryptography transitive dependency

Fixes #52

Somewhere in the dependency tree, cryptography is being installed.
In the recent 2.1 release of the cryptography package, Python 2.6
was deprecated.

Fixes #52

Somewhere in the dependency tree, cryptography is being installed.
In the recent 2.1 release of the cryptography package, Python 2.6
was deprecated.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.692% when pulling 7a71518 on shakeelmohamed:ci/fix-appveyor-python2.6 into 8c52a67 on mayn:master.

@mayn
Copy link
Owner

mayn commented Oct 12, 2017

@shakeelmohamed thanks for your PR!
i saw that also... lets see what appveyor thinks.
nice write-up.

@shakeelmohamed
Copy link
Contributor Author

@mayn Now Python 3.3 fails to build the cryptography package! 😄 Is there a way to conditionally set this restriction for Python 2.6 only?

@mayn
Copy link
Owner

mayn commented Oct 12, 2017

@shakeelmohamed didn't see that one coming. 😄
Assumingly you should be able to implement an conditional pin for python 2.6 appveyor environment for the tox profile (https://github.com/mayn/packerlicious/blob/master/tox.ini#L21)

Might require minor refactor to the tox profile for appveyor also (not 100% sure)

see:
http://tox.readthedocs.io/en/latest/config.html#generating-environments-conditional-settings

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.692% when pulling 12b11f2 on shakeelmohamed:ci/fix-appveyor-python2.6 into 8c52a67 on mayn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.692% when pulling 256ae95 on shakeelmohamed:ci/fix-appveyor-python2.6 into 8c52a67 on mayn:master.

@shakeelmohamed
Copy link
Contributor Author

shakeelmohamed commented Oct 12, 2017

This solution works, but it's a bit clunky since it bring in another requirements file. Changing the tox config didn't seem to work, I'm guessing that's because pip install -r requirements-travis.txt was happening before tox in both CI scenarios. (So, it didn't matter which version of cryptography was specified in the tox file since it was already failing to install v2.1 before that point)

One possible solution would be moving the CI installations to tox.ini, but that could add some bloat for local devs - a worthwhile tradeoff perhaps.

Copy link
Owner

@mayn mayn left a comment

Choose a reason for hiding this comment

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

hey this is great.
Given that this is only for the build environments and tox -e py26 --recreate still works locally for users, I can live with the clunk.

@mayn mayn merged commit fce572e into mayn:master Oct 12, 2017
@mayn mayn added this to the 0.5.0 milestone Oct 12, 2017
@shakeelmohamed shakeelmohamed deleted the ci/fix-appveyor-python2.6 branch October 13, 2017 03:50
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