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.

Sign up for GitHub

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

Jump to bottom

Install securedrop-app-code dependencies in development environment via make venv #6358

Closed
legoktm opened this issue Mar 23, 2022 · 3 comments · Fixed by #6351
Closed

Install securedrop-app-code dependencies in development environment via make venv #6358

legoktm opened this issue Mar 23, 2022 · 3 comments · Fixed by #6351
Assignees
@legoktm

Comments

@legoktm
Copy link
Member

legoktm commented Mar 23, 2022 •
edited
Loading

Description

We should install the securedrop-app-code-requirements.txt dependencies when setting up the development environment via make venv.

The main motivation for this is to make mypy work better out of the box. Currently we use an old version of mypy that bundles possibly-outdated type stubs for key libraries we use like flask. Because new mypy (as planned in #6351) no longer bundles those stubs, it'll be impossible to use mypy locally unless you also have flask, etc. installed in your development virtualenv.

In addition this will make using editors like PyCharm much easier, because it expects all the dependencies to be installed in a single virtualenv.

Finally, this should bring the local development environment closer to what CI does, which AIUI takes a venv with securedrop-app-code-requirements deps installed, and then installs develop-requirements over it.

The only obstacle to this is that right now develop wants click==7, while sd-app-code wants click==8. This appears to be easily fixable, since our molecule version is already compatible with click 8.
@legoktm
Copy link
Member Author

legoktm commented Mar 23, 2022

This is a real rabbit hole...

  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [1 lines of output]
      error in jsmin setup command: use_2to3 is invalid.
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.

This is because if you create a venv, it has latest setuptools, which is incompatible with our old jsmin, and setuptools only gets downgraded later on.

Stupid fix: install develop-reqs first, which downgrades setuptools, then install sd-app-code which has jsmin.

Better fix: have boot-strap-venv.sh downgrade setuptools to 56 before we install anything else, at the cost of duplicating setuptools==56.

@legoktm
Copy link
Member Author

legoktm commented Mar 23, 2022

Other option: just include requirements/python3/securedrop-app-code-requirements.in in develop-requirements.txt 🤔

Trying that now...

legoktm added a commit that referenced this issue Mar 23, 2022
@legoktm
Include securedrop-app-code deps into develop-requirements.txt
07f82d1 
We want the development virtualenv to include the production dependencies
as well as the development ones for the benefit of tools like mypy as well
as editors like PyCharm. Care was taken to ensure all the versions match
those used in production.

The main change to develop-requirements.txt is that click has been upgraded
to v8, because Flask v2 requires it.

We also have `make venv` downgrade setuptools before we install anything,
this is mostly for the benefit of `jsmin` which uses now-removed setuptools
functionality and requires an older version just to be installable.

Fixes #6358.
@legoktm legoktm mentioned this issue Mar 23, 2022
Closed
8 tasks
@legoktm legoktm self-assigned this Mar 23, 2022
@legoktm legoktm mentioned this issue Mar 24, 2022
Merged
2 tasks
@legoktm
Copy link
Member Author

legoktm commented Mar 31, 2022

Today we discussed that rather than targetting develop-requirements, we'll put mypy in test-requirements.txt and run it via the dev container.

legoktm added a commit that referenced this issue Mar 31, 2022
@legoktm
Upgrade mypy and enable SQLAlchemy plugin
bacc485 
* Upgrade mypy to 0.942 and sqlalchemy-stubs to 0.4. Type stubs are no
  longer bundled with mypy, so we need to explicitly install various
  types-{name} packages.
* Since we need to install extra packages and need other dependencies
  like Flask, move mypy into test-requirements.txt and run it via the
  dev container.
* The main issue spotted by the SQLAlchemy plugin, that
  sources.filesystem_id is nullable, is being fixed in a separate PR:
  #6350.
* Drop the explicit type comments now that mypy can infer them.
* Use `--explicit-package-bases` when running mypy, so it treats the
  securedrop/admin directories as modules regardless of whether
  `__init__.py` files exist. This is needed so the two tests/
  directories don't conflict with each other.
* Add missing return types to all schema changes and the templates that
  generate them.
* In the `create_source_uuid_column` migration, drop the use of
  `quoted_name()`. For some reason mypy doesn't like it, but more
  importantly we don't need it in SQLite and we don't use it in any
  other schema change.
* Make SecureTemporaryFile.write()'s return type match its parent by
  returning the number of bytes that were written.

Fixes #6346.
Fixes #6358.
legoktm added a commit that referenced this issue Mar 31, 2022
@legoktm
Upgrade mypy and enable SQLAlchemy plugin
d3eb605 
* Upgrade mypy to 0.942 and sqlalchemy-stubs to 0.4. Type stubs are no
  longer bundled with mypy, so we need to explicitly install various
  types-{name} packages.
* Since we need to install extra packages and need other dependencies
  like Flask, move mypy into test-requirements.txt and run it via the
  dev container.
* The main issue spotted by the SQLAlchemy plugin, that
  sources.filesystem_id is nullable, is being fixed in a separate PR:
  #6350.
* Drop the explicit type comments now that mypy can infer them.
* Use `--explicit-package-bases` when running mypy, so it treats the
  securedrop/admin directories as modules regardless of whether
  `__init__.py` files exist. This is needed so the two tests/
  directories don't conflict with each other.
* Add missing return types to all schema changes and the templates that
  generate them.
* In the `create_source_uuid_column` migration, drop the use of
  `quoted_name()`. For some reason mypy doesn't like it, but more
  importantly we don't need it in SQLite and we don't use it in any
  other schema change.
* Make SecureTemporaryFile.write()'s return type match its parent by
  returning the number of bytes that were written.

Fixes #6346.
Fixes #6358.
legoktm added a commit that referenced this issue Apr 7, 2022
@legoktm
Upgrade mypy and enable SQLAlchemy plugin
516fd35 
* Upgrade mypy to 0.942 and sqlalchemy-stubs to 0.4. Type stubs are no
  longer bundled with mypy, so we need to explicitly install various
  types-{name} packages.
* Since we need to install extra packages and need other dependencies
  like Flask, move mypy into test-requirements.txt and run it via the
  dev container.
* The main issue spotted by the SQLAlchemy plugin, that
  sources.filesystem_id is nullable, is being fixed in a separate PR:
  #6350.
* Drop the explicit type comments now that mypy can infer them.
* Use `--explicit-package-bases` when running mypy, so it treats the
  securedrop/admin directories as modules regardless of whether
  `__init__.py` files exist. This is needed so the two tests/
  directories don't conflict with each other.
* Add missing return types to all schema changes and the templates that
  generate them.
* In the `create_source_uuid_column` migration, drop the use of
  `quoted_name()`. For some reason mypy doesn't like it, but more
  importantly we don't need it in SQLite and we don't use it in any
  other schema change.
* Make SecureTemporaryFile.write()'s return type match its parent by
  returning the number of bytes that were written.

Fixes #6346.
Fixes #6358.
legoktm added a commit that referenced this issue Apr 12, 2022
@legoktm
Upgrade mypy and enable SQLAlchemy plugin
54c921b 
* Upgrade mypy to 0.942 and sqlalchemy-stubs to 0.4. Type stubs are no
  longer bundled with mypy, so we need to explicitly install various
  types-{name} packages.
* Since we need to install extra packages and need other dependencies
  like Flask, move mypy into test-requirements.txt and run it via the
  dev container.
* The main issue spotted by the SQLAlchemy plugin, that
  sources.filesystem_id is nullable, is being fixed in a separate PR:
  #6350.
* Drop the explicit type comments now that mypy can infer them.
* Use `--explicit-package-bases` when running mypy, so it treats the
  securedrop/admin directories as modules regardless of whether
  `__init__.py` files exist. This is needed so the two tests/
  directories don't conflict with each other.
* Add missing return types to all schema changes and the templates that
  generate them.
* In the `create_source_uuid_column` migration, drop the use of
  `quoted_name()`. For some reason mypy doesn't like it, but more
  importantly we don't need it in SQLite and we don't use it in any
  other schema change.
* Make SecureTemporaryFile.write()'s return type match its parent by
  returning the number of bytes that were written.

Fixes #6346.
Fixes #6358.
legoktm added a commit that referenced this issue Apr 19, 2022
@legoktm
Upgrade mypy and enable SQLAlchemy plugin
9a7386f 
* Upgrade mypy to 0.942 and sqlalchemy-stubs to 0.4. Type stubs are no
  longer bundled with mypy, so we need to explicitly install various
  types-{name} packages.
* Since we need to install extra packages and need other dependencies
  like Flask, move mypy into test-requirements.txt and run it via the
  dev container.
* The main issue spotted by the SQLAlchemy plugin, that
  sources.filesystem_id is nullable, is being fixed in a separate PR:
  #6350.
* Drop the explicit type comments now that mypy can infer them.
* Use `--explicit-package-bases` when running mypy, so it treats the
  securedrop/admin directories as modules regardless of whether
  `__init__.py` files exist. This is needed so the two tests/
  directories don't conflict with each other.
* Add missing return types to all schema changes and the templates that
  generate them.
* In the `create_source_uuid_column` migration, drop the use of
  `quoted_name()`. For some reason mypy doesn't like it, but more
  importantly we don't need it in SQLite and we don't use it in any
  other schema change.
* Make SecureTemporaryFile.write()'s return type match its parent by
  returning the number of bytes that were written.

Fixes #6346.
Fixes #6358.
@legoktm legoktm mentioned this issue May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@legoktm legoktm

Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@legoktm