[15.0][REV][FIX] Pin cryptography to keep compatibility with Odoo's pyopenssl==19.0.0 #3286
Conversation
…sl==19.0.0" Reverted commit disallows to install auto_backup, when using offical Odoo 15.0 docker image, which is based on bullseye [1]. The bullseye has following python packages: - cryptography 3.3.2 with security patches [2] - pyopenssl 20.0.1 [3] [1] https://github.com/odoo/docker/blob/5fb6a842747c296099d9384587cd89640eb7a615/15.0/Dockerfile#L1 [2] https://packages.debian.org/bullseye/python3-cryptography [3] https://packages.debian.org/bullseye/python3-openssl
|
Hello, @sbidoul! Bumping you as maintainer of oca-ci. Could you please install packages from https://github.com/odoo/odoo/blob/15.0/requirements.txt and then install odoo itself? |
There was a problem hiding this comment.
There still has to be some sort of pin if we don't want to run into odoo/odoo#99829 again. Have a look OCA/server-auth#429
server-auth is other repository and I don't see problems with pining cryptography<37 in that repository. |
First |
|
Given that @sbidoul is actually a maintainer of Python's pip I would say he's among those of us in the OCA that is most knowledgeable about the installation of python requirements. Maybe you can reconsider adopting his own approach to this problem (in OCA/server-auth#429). |
a39cd21 to
805393c
Compare
|
Stéphane Bidoul, sorry for pinging for hasty reasons. In oca-ci packages are already installed like I assumed and asked. |
805393c to
f622afc
Compare
|
For the record, my suggestion is to follow OCA/server-auth#429 in adding '<37' in requirements.txt, not in test-requirements.txt. |
requirements.txt is autogenerated from module. Adding "cryptography<37" to auto_backup manifest is not correct. It is not auto_backups' scope to deal with odoo's subsubdependency. |
Pardon. It is not auto_backups' scope to deal with its own subsubdependency. For that we already have requirements.txt from odoo/odoo or OCA/OCB. |
|
I wrote an explanation of the root cause of a similar problem in OCA/server-auth#424 (comment) I also agree an exact pin must absolutely be avoided.
I tend to agree with that. And TBH I don't quite remember right now why I put the upper bound in the manifest in OCA/server-auth#429. I can see a benefit for some users, but also a downside for users who would want to use cryptography>=37 and take care of installing a compatible pyopenssl. So I'm +1 with putting the upper bound on cryptography in BTW, these topics are super complex. For instance |
@StefanRijnhart as I don't remember why I preferred that too in 2022 and now putting it in test-requirements.txt sounds somewhat better to me, do you have specific arguments in mind yourself? |
…ng github actions
f622afc to
71a3a3c
Compare
There was a problem hiding this comment.
@sbidoul While it not being auto_backup's responsibility, as per OCA's implementation the dependency from the manifest with or without version ends up in requirements.txt, and this will be the basis for many deploys, either directly or indirectly, for the installation of these dependencies. I consider therefore adding the version to auto_backup's manifest as a valid workaround (or helpful guidance) for the indeed 'super complex' issue of preventing the breaking version of cryptography from being installed on any instance.
Then again, I respect your knowledge and authority on the issue so I will retract my blocking review and may abstain from further reviews.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Reverted commit disallows to install auto_backup, when using offical Odoo 15.0 docker image, which is based on bullseye [1].
The bullseye has following python packages:
[1] https://github.com/odoo/docker/blob/5fb6a842747c296099d9384587cd89640eb7a615/15.0/Dockerfile#L1
[2] https://packages.debian.org/bullseye/python3-cryptography
[3] https://packages.debian.org/bullseye/python3-openssl