Repository navigation
Loosen or update cryptography requirement for dash[testing] #1466
Description
Thank you so much for helping improve the quality of Dash!
We do our best to catch bugs during the release process, but we rely on your help to find the ones that slip through.
Describe your context
dash[testing] has Cryptography hardcoded to 3.0 (cryptography==3.0). Can we either get this loosened or updated to 3.2.1 as cryptography > 3.2 has a vulnerability.
Describe the bug
dash[testing] has Cryptography hardcoded to 3.0 (cryptography==3.0).
Expected behavior
Can we either get this loosened or updated to 3.2.1 as cryptography > 3.2 has a vulnerability.
Reacted by Anders Fredrik Kiær, Alex Johnson and Ned Letcher
Activity
anders-kiaer commentedon Nov 26, 2020
Semi-related to #1457 (wrt. generally loosening
pip install dash[testing]dependency versions).alexcjohnson commentedon Nov 30, 2020
We can certainly update this requirement (PRs welcome 😄)
Loosening these requirements is also a good idea, but will take a bit more effort. As @anders-kiaer deduced in #1457, the purpose of pinning was originally for our own reproducibility in CI testing, so if we are to loosen these requirements we need to either simultaneously pin them in the CI setup for
dashand our other repos that use it (the coredash-core-components,dash-html-components,dash-table, and othersdash-bio,dash-daq, etc) or find a good way to respond to any failures that might creep in due to dep upgrades.If we did decide to pin internally but loosen
[testing], that could take the form of another requirements file in the maindashrepo that's used just for this purpose, or perhaps we just pack the pinned versions of all of these into[dev], so that[testing]is for use by other projects and internally we only use[dev]? The problem I see with that is that we won't know about problems in[testing].anders-kiaer commentedon Dec 1, 2020
dash+dash[testing]probably have more indirect Python dependencies than direct dependencies. I.e. even if we pin the direct test dependencies, CI failures will happen due to indirect dependency upgrades. A recent example in our project earlier this autumn waspylintfailure due to newastroidversion (even though there was newpylintversion released on PyPI at that time).If you want to pin all dependencies (indirect + direct) maybe
pipenvcould be an option to use during CI, and let it fail when lock file is out of sync. My experience withpipenvis limited, especially in CI setting. You will still have the challenge as you mention @alexcjohnson that CI then would not see challenges users meet - i.e. you would maybe like to double the number of CI jobs (pinned + unpinned, and let the unpinned jobs be allowed to fail).In our Dash based project, we have gone for a much simpler mitigation route: We have scheduled CI runs (once every night), and also
pip freezeas part of the CI run. This way maintainers + community immediately will see when a new upstream dependency breaks something (maintainers could also get an email notification regarding failed scheduledmaster). Quick notification, and the possibility of comparing with last success'pip freezequickly identify the new release (either direct or indirect dependency) causing the failure, and we are able to also notify the upstream dependency quickly after they new release (which they typically are grateful for, since the dependencies in question here are typically well maintained and with long deprecation cycles, i.e. a new release causing a failure is often unintentional and not a sudden breaking change).I'm not familiar with Circle CI (and actually had some trouble getting Circle CI to start in my Dash PRs). Is it correct that PR authors would need a Circle CI account to be able to start and/or see runs? We recently jumped over to GitHub actions and happy with it so far (example of scheduled runs in a Dash component repository). Not sure if GitHub actions is something that has been considered in Dash, but I'm happy to contribute with some CI workflow suggestions if that is the case. Then #1440 can maybe be addressed at the same time if found useful (automatic deploy of
sdist+wheelsto PyPI when Dash maintainers do a release on GitHub).Side note: I just observed that GitHub apparently does not pick up the
requires-*.txtfiles in this repository. At least I can not see them in https://github.com/plotly/dash/network/dependencies. Might be that it picks uprequirements-*.txt, and thereby give maintainers of Dash the option of being notified regarding dependency security advisories (like in the issue reported here).ned2 commentedon Dec 3, 2020
This is perhaps something of a more pressing concern as it seems that
dash[testing]just on its own now has an unresolvable set of dependency constraints since pyopenssl recently bumped tocryptography>=3.2in v20.0.0 released on November 28.More generally, I agree that it would be great if
dash[testing]could be unpinned/loosened. As @anders-kiaer mentions in #1457,dash[testing]is advertised as a (super useful) public API, and pinning runs counter to that purpose.alexcjohnson commentedon Dec 16, 2020
@anders-kiaer that seems like the winning argument to me 🏆 especially with the recent pip resolver changes.
So we'll loosen all the requirements in
[testing]to>=We'll keep
[dev]requirements==because this is used solely (AFAIK) by our own CI - on this repo, the rest of the dash core, and other dash components such asdash-daqanddash-bio.alexcjohnson commentedon Dec 16, 2020
Just for future reference:
cryptographyis indeed one of these indirect dependencies, it's not used directly at all. We pinned it in the summer due to a windows bug, see #1384 (comment). Looks like that has been fixed so hopefully now we can simply remove it again from our requirements 🤞