Make Python 3 the default for development #4544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmol
requested review from
conorsch,
emkll,
heartsucker,
kushaldas,
msheiny,
redshiftzero and
zenmonkeykstop
as code owners
kushaldas
suggested changes
Jun 20, 2019
There was a problem hiding this comment.
$ make lint
███ Preparing Python 3 virtual environment...
Default virtualenv in .venv is Python 2; renaming it to .venv-2-20190620-123047.
Already using interpreter /usr/bin/python3
Using base prefix '/usr'
New python executable in /home/kdas/code/securedrop/.venv/bin/python3
Also creating executable in /home/kdas/code/securedrop/.venv/bin/python
Installing setuptools, pip, wheel...done.
███ Linting Ansible configuration...
--> Validating schema /home/kdas/code/securedrop/molecule/virtualbox-staging-xenial/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/fetch-tor-packages/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/builder-xenial/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/qubes-staging/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/ansible-config/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/upgrade/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/libvirt-staging-xenial/molecule.yml.
Validation completed successfully.
--> Validating schema /home/kdas/code/securedrop/molecule/vagrant-packager/molecule.yml.
Validation completed successfully.
--> Test matrix
└── ansible-config
└── verify
--> Inventory /home/kdas/code/securedrop/molecule/ansible-config/../../install_files/ansible-base/host_vars linked to /tmp/molecule/securedrop/ansible-config/inventory/host_vars
--> Inventory /home/kdas/code/securedrop/molecule/ansible-config/../../install_files/ansible-base/group_vars linked to /tmp/molecule/securedrop/ansible-config/inventory/group_vars
--> Scenario: 'ansible-config'
--> Action: 'verify'
Traceback (most recent call last):
File "/home/kdas/code/securedrop/.venv/bin/molecule", line 10, in <module>
sys.exit(main())
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/core.py", line 722, in __call__
return self.main(*args, **kwargs)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/core.py", line 697, in main
rv = self.invoke(ctx)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/core.py", line 1066, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/core.py", line 895, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/core.py", line 535, in invoke
return callback(*args, **kwargs)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/click/decorators.py", line 17, in new_func
return f(get_current_context(), *args, **kwargs)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/molecule/command/verify.py", line 97, in verify
base.execute_subcommand(scenario.config, action)
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/molecule/command/base.py", line 102, in execute_subcommand
return command(config).execute()
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/molecule/command/verify.py", line 72, in execute
self._config.verifier.execute()
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/molecule/verifier/testinfra.py", line 184, in execute
self.bake()
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/molecule/verifier/testinfra.py", line 163, in bake
self._testinfra_command = sh.Command('py.test').bake(
File "/home/kdas/code/securedrop/.venv/lib/python3.5/site-packages/sh.py", line 1202, in __init__
raise CommandNotFound(path)
sh.CommandNotFound: py.test
Makefile:76: recipe for target 'ansible-config-lint' failed
make: *** [ansible-config-lint] Error 1
13f06a7 to
af075c2
Compare
conorsch
reviewed
Jun 20, 2019
redshiftzero
suggested changes
Jun 21, 2019
Codecov Report
@@ Coverage Diff @@
## develop #4544 +/- ##
==========================================
+ Coverage 82.53% 82.63% +0.1%
==========================================
Files 45 45
Lines 3121 3116 -5
Branches 339 337 -2
==========================================
- Hits 2576 2575 -1
+ Misses 458 455 -3
+ Partials 87 86 -1
Continue to review full report at Codecov.
|
rmol
force-pushed
the
4519-py3-dev
branch
4 times, most recently
from
d16bc15 to
71a308d
Compare
Make the default .venv Python 3, moving existing Python 2 versions out of the way if necessary. Compile and use separate requirements files for each version, using .venv2 to update the Python 2 requirements. Add Makefile targets to make it easier to create both virtualenvs; devops/scripts/boot-strap-venv.sh can now be sourced as before or just run to create and update the virtualenv. Merge Makefile and securedrop/Makefile. I've left securedrop/Makefile in place with targets echoing deprecation warnings, then invoking their equivalents in the top-level Makefile. Type linting no longer requires a dedicated virtualenv once we're on Python 3, so mypy is now part of the normal development requirements. Switch mypy checking from 2.7 to Python 3.5. Our Python dependencies also required updates, starting with Molecule, which didn't support Python 3.5 until 2.20.1, so would have required us to run a more recent Python 3 for development than we deploy under. The pip-tools package needed to be updated to work with pip 19.1.1, which is installed in the new Python 3 virtualenv. Fix many, many lint errors from lint packages updated for Python 3.
Make sure lint targets activate the virtualenv as necessary to pick up pytest, since not everyone will have it somewhere else on the path. Ahem. Normalize lint target between Makefiles. Rename some variables in the top-level Makefile. Circle's still running the EOL Ubuntu 14.04, which has Python 3.4. We can run most linters under 3.5 in our 16.04 container, but the shellcheck package there is too old, so unless we want to run Docker-in-Docker, we need to run shellcheck in a separate step using a koalaman image.
When run in a non-interactive shell, virtualenv < 16.2 can cause an error because PS1 is unset. Apparently whatever's in the image used for fetch-tor-packages is that old.
I misunderstood a problem raised in another context; this isn't necessary.
Added pip-tools to our Ubuntu Dockerfiles, to get requirements compiled with the Python versions that will be used in production.
rmol
force-pushed
the
4519-py3-dev
branch
2 times, most recently
from
347fff1 to
f76e6c8
Compare
Installing pip requirements automatically is bad, and fails in the container, outside of a virtualenv. Instead, if safety or bandit is not available, request installation. Also improve the test-config/config.py rules to only build the test config when necessary.
redshiftzero
approved these changes
Jun 29, 2019
There was a problem hiding this comment.
I've ran through the following targets on this branch, and am happy with merging this:
✅make test
✅PYTHON_VERSION=2 make test
✅make lint
✅make update-pip-requirements
✅make update-python2-requirements
✅make build-debs and was able to provision staging (using virtualbox)
We can create a followup issue on this sprint for broader developer docs changes once this is merged.
@kushaldas please give this another look and update your review, and then feel free to merge if you also think this is ready (a second set of eyes given the diff size is important here).
| @@ -0,0 +1,200 @@ | |||
| # | |||
There was a problem hiding this comment.
Noting here for other maintainers the requirements diff compared to securedrop-app-code-requirements.txt on develop - this PR only removes dependencies from the python 3 version of the app code requirements:
#
alembic==0.9.9 \
--hash=sha256:85bd3ea7633024e4930900bc64fb58f9742dedbc6ebb6ecf25be2ea9a3c1b32e
@@ -96,12 +96,6 @@ cryptography==2.6.1 \
--hash=sha256:d4afbb0840f489b60f5a580a41a1b9c3622e08ecb5eec8614d4fb4cd914c4460 \
--hash=sha256:d9ed28030797c00f4bc43c86bf819266c76a5ea61d006cd4078a93ebf7da6bfd \
--hash=sha256:e603aa7bb52e4e8ed4119a58a03b60323918467ef209e6ff9db3ac382e5cf2c6
-enum34==1.1.6 \
- --hash=sha256:2d81cbbe0e73112bdfe6ef8576f2238f2ba27dd0d55752a776c41d38b7da2850 \
- --hash=sha256:644837f692e5f550741432dd3f223bbb9852018674981b1664e5dc339387588a \
- --hash=sha256:6bd0f6ad48ec2aa117d3d141940d484deccda84d4fcd884f5c3d93c23ecd8c79 \
- --hash=sha256:8ad8c4783bf61ded74527bffb48ed9b54166685e4230386a9ed9b1279e2df5b1 \
- # via argon2-cffi, cryptography
flask-assets==0.12 \
--hash=sha256:6031527b89fb3509d1581d932affa5a79dd348cfffb58d0aef99a43461d47847
flask-babel==0.11.2 \
@@ -116,10 +110,6 @@ flask-wtf==0.14.2 \
flask==1.0.2 \
--hash=sha256:2271c0070dbcb5275fad4a82e29f23ab92682dc45f9dfbc22c02ba9b9322ce48 \
--hash=sha256:a080b744b7e345ccfcbc77954861cb05b3c63786e93f2b3875e0913d44b43f05
-ipaddress==1.0.22 \
- --hash=sha256:64b28eec5e78e7510698f6d4da08800a5c575caa4a286c93d651c5d3ff7b6794 \
- --hash=sha256:b146c751ea45cad6188dd6cf2d9b757f6f4f8d6ffb96a023e6f2e26eea02a72c \
- # via cryptography
itsdangerous==0.24 \
--hash=sha256:cbb3fcf8d3e33df861709ecaf89d9e6629cff0a217bc2848f1b41cd30d360519 \
# via flask
kushaldas
approved these changes
Jul 2, 2019
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
Make the default .venv Python 3, moving existing Python 2 versions out
of the way if necessary. Compile and use separate requirements files
for each version, using .venv2 to update the Python 2 requirements.
Add Makefile targets to make it easier to create both virtualenvs;
devops/scripts/boot-strap-venv.sh can now be sourced as before or just
run to create and update the virtualenv.
Merge Makefile and securedrop/Makefile. I've left securedrop/Makefile
in place with targets echoing deprecation warnings, then invoking
their equivalents in the top-level Makefile.
Type linting no longer requires a dedicated virtualenv once we're on
Python 3, so mypy is now part of the normal development requirements.
Switch mypy checking from 2.7 to Python 3.5.
Our Python dependencies also required updates, starting with Molecule,
which didn't support Python 3.5 until 2.20.1, so would have required
us to run a more recent Python 3 for development than we deploy under.
The pip-tools package needed to be updated to work with pip 19.1.1,
which is installed in the new Python 3 virtualenv.
Fix many, many lint errors from lint packages updated for Python 3.
Fixes #4519.
Fixes #4415.
Testing
Try running various Makefile targets:
make lintmake testPYTHON_VERSION=2 make testmake update-pip-requirementsmake update-python2-requirementsmake build-debs(you should only get the usual apt update test failure)make stagingmolecule verify -s libvirt-stagingDeployment
This should not include any changes to production deployment.
Debian packages are still built with Python 2, and securedrop-app-code is still using the Python 2 version of libapache2-mod-wsgi. The only changes to building debs were to copy the new requirement file paths and specify that the Python 2-specific ones should be used when building wheels.
The only changes to the admin were to satisfy linters, and tests are passing.
Checklist
If you made changes to the server application code:
make lint) and tests (make -C securedrop test) pass in the development containerIf you made changes to
securedrop-admin:make -C admin test) pass in the admin development containerIf you made changes to the system configuration:
If you made non-trivial code changes:
If you made changes to documentation:
make docs-lint) passed locally