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

saw-remote-api: Review whether dependencies listed in requirements.txt are needed #1152

Closed
RyanGlScott opened this issue Mar 25, 2021 · 3 comments
Labels
subsystem: saw-remote-api Issues related to the SAW server and its RPC bindings

Comments

@RyanGlScott
Copy link
Contributor

The requirements.txt file lists a surprising number of dependencies:

alabaster==0.7.12
argo-client==0.0.3
Babel==2.8.0
BitVector==3.4.9
certifi==2020.6.20
chardet==3.0.4
docutils==0.16
idna==2.10
imagesize==1.2.0
Jinja2==2.11.2
MarkupSafe==1.1.1
mypy==0.790
mypy-extensions==0.4.3
packaging==20.4
Pygments==2.7.1
pyparsing==2.4.7
pytz==2020.1
requests==2.24.0
six==1.15.0
snowballstemmer==2.0.0
Sphinx==3.2.1
sphinxcontrib-applehelp==1.0.2
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-htmlhelp==1.0.3
sphinxcontrib-jsmath==1.0.1
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-serializinghtml==1.1.4
typed-ast==1.4.2
typing-extensions==3.7.4.3
urllib3==1.25.10

Some of these dependencies, such as Jinja2, aren't listed in the poetry.lock file generated by poetry, which computes the transitive closure of the dependencies needed in the pyproject.toml file. This leads me to wonder if we can remove some of the dependencies in requirements.txt, especially since we semi-frequently receive notifications from @dependabot to upgrade things like Jinja2.

@RyanGlScott RyanGlScott added the subsystem: saw-remote-api Issues related to the SAW server and its RPC bindings label Mar 25, 2021
@pnwamk
Copy link
Contributor

pnwamk commented Mar 25, 2021

I also don't see Sphinx et al in the poetry.lock, which we've used to automatically generate documentation, some of which is/can be formatted in HTML. Still not sure the root cause, but I suspect whatever it is might explain both these things.

@RyanGlScott
Copy link
Contributor Author

I also don't see Sphinx et al in the poetry.lock, which we've used to automatically generate documentation, some of which is/can be formatted in HTML.

This is a good point, and a nuance that I didn't really convey when submitting this issue. Namely, not all of the dependencies listed in the requirements.txt file are actually needed to build the core saw package itself. Rather, some of these dependencies are only used for developer-only workflows, such as generating Sphinx documentation. Poetry calls these dev-dependencies (see cryptol's pyproject.toml file for an example), and indeed, the pyproject.toml files for cryptol and saw should probably include Sphinx et al. as dev-dependencies.

Still, I'm unclear if jinja2 is included in service of a developer-specific workflow. I could believe that jinja2 is a dependency of some other library in the requirements.txt file, but if that's the case, it seems like it should be safe to remove it, since installing the other library would bring in jinja2 anyway.

As a side note, this is one of the reasons I prefer using Poetry to manage dependencies over requirements.txt, as Poetry can figure out a lot of this guesswork for you. I recognize the importance of keeping requirements.txt files for the sake of those who would prefer to just install a virtualenv instead of using fancy tools, but still.

@RyanGlScott
Copy link
Contributor Author

As of 809a39c, the requirements.txt file is now auto-generated based on the contents of pyproject.toml, so the two files are now in sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: saw-remote-api Issues related to the SAW server and its RPC bindings
Projects
None yet
Development

No branches or pull requests

2 participants