Merge lp://staging/~twom/conn-check/py3k into lp://staging/conn-check

Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: 149
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp://staging/~twom/conn-check/py3k
Merge into: lp://staging/conn-check
Diff against target: 488 lines (+132/-67)
13 files modified
Makefile (+3/-1)
conn_check/check_impl.py (+2/-1)
conn_check/checks.py (+15/-6)
conn_check/main.py (+40/-37)
conn_check/patterns.py (+4/-2)
conn_check/utils/convert_fw_rules.py (+3/-2)
conn_check/utils/firewall_rules.py (+2/-1)
conn_check/version.txt (+1/-1)
requirements.txt (+3/-2)
setup.py (+24/-9)
test-requirements.txt (+1/-0)
tests.py (+17/-5)
tox.ini (+17/-0)
To merge this branch: bzr merge lp://staging/~twom/conn-check/py3k
Reviewer Review Type Date Requested Status
Tom Wardill (community) Approve
Simon Davy (community) Approve
Review via email: mp+336597@code.staging.launchpad.net

Commit message

Port to python3 using futurize. Refactor to allow import and use as a library.

Description of the change

Use `future` to allow execution in a python3 environment.

Use `tox` for running tests under python2.7 and any available python3 environment.

To post a comment you must log in.
Revision history for this message
Simon Davy (bloodearnest) :
review: Approve
Revision history for this message
Tom Wardill (twom) :
Revision history for this message
Tom Wardill (twom) wrote :

Fixes after review.

review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :
lp://staging/~twom/conn-check/py3k updated
149. By Tom Wardill

Update pip before installing

Revision history for this message
Daniel Manrique (roadmr) wrote :

About the build failure you had earlier. It was because the version of pip is too ancient to deal with cryptography >=2.1, which you identified correctly. And you're updating pip prior to performing the asset build. However, in the deployed servers, the old version of pip is still installed system-wide, and we usually don't update pip in the venv because the system pip has some Ubuntu-specific changes having to do with how it deals with non-system wheels (IIRC upstream pip refuses to install a wheel if a system package provides that module, where Ubuntu-hacked pip does install and use the given wheel, unless you specify using system packages - it was something like that).

Instead, what we usually do is specify the latest version of the wheel that still works with the old pip. In cryptography's case it's 2.0.3. So you could update requirements.txt which currently says cryptography >=0.5,to say cryptography == 2.0.3 (essentially pinning it to avoid later surprises).

This is a bizarre way to handle things but has to do with our old Trusty-based deployments. Once we manage to migrate the world to Xenial or later, this issue will go away (though since Xenial has a newer, nicer pip, again the consequence is that updating pip should not happen).

This merge is approved and processing, so I would say keep an eye on things and see how they behave, but if pip-related trouble ensues, I'd recommend rolling this back and downgrading cryptography instead.

Revision history for this message
Tom Wardill (twom) wrote :

That is useful info and something I wasn't aware of, thanks.
I can potentially see (though I've not checked) a problem where we need newer versions for the python 3 support vs the version supported by pip. Hopefully not in this case.

Is another option might be to specify the older systems (trusty) get the previous version (1.3.1 or 1.3.2, depending on which source of version information you look at)?
There's no functionality changes between the versions, the changes here were made to support snapstore-snap.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches