search

pulp / pulp-certguard

Pulp plugin that provides an x.509 certificate based content-guard.
GNU General Public License v2.0
3 stars 26 forks source link

Update requirements.txt #211

Closed m-bull closed 1 year ago

m-bull commented 1 year ago

Use a more stringent upper version constraint on PyOpenSSL to ensure that cryptography can be installed at <38.0.0. Only required as a temporary fix while pulpcore has a dependency on cryptography~=37.0.4

Follows issue here https://github.com/pulp/pulpcore/issues/3269.

bmbouter commented 1 year ago

Recently we've changed our pulpcore declares dependencies for instance on main branch it looks like this now.

If I'm understanding right that this change is entirely due to cryptography~=37.0.4 being on pulpcore, can we adjust the dependency declaration on pulpcore instead? Which pulpcore versions would need to have the broader dep declaration?

bmbouter commented 1 year ago

Another option is to widen the dependency here. Generally we're asking all plugins to widen their dep ranges so that would be also ok.

ggainey commented 1 year ago

@bmbouter as I understand it, the issue here is that current-released-pulpcore wants cryptography ~=37.0.4, certguard pulls in PyOpenSSL 22.1.0 (the new thing that has invoked this issue) which requires cryptography-38, and therefore certguard and core suddenly can't be installed together. If we backport the pulpcore-requirement-change to released branches, then this isn't needed. I think?

As far as broadening the range - right now, the only things certguard requires are "PyOpenSSL<23.0 , pulpcore>=3.10,<3.25" - which is pretty broad already, yeah?

mdellweg commented 1 year ago

I'm missing the point how tightening one requirement should enable any installation at all. Sure pip is weird, but imho this can only make more combinations impossible.

bmbouter commented 1 year ago

@m-bull I think we're looking to you for the next step. I'm very interesting in seeing this resolved. Please let us know how we can help.

m-bull commented 1 year ago

Apologies for being slow, have been following the discussion and wanted to put a bit more meat on the issue to help @mdellweg with context.

mdellweg commented 1 year ago

I want to see this solved in pulpcore. Surely pip(-compile) has issues finding all the possible combinations, but being less strict on the pulpcore side is the way to go. What we really need is the (pulpcore) y-stream releases we need this change to appear on.

m-bull commented 1 year ago

No problem! Issue is here: https://github.com/pulp/pulp-certguard/issues/212 with the output from pulp_installer, which should hopefully aid debugging on the pulpcore/pulp_installer sides.

I don't mind whether this gets merged or not - I got over our Pulp upgrade blockers by installing a fork of pulp-certguard with this patch applied, and I'm happy to wait for this to shake out before the next upgrade...

stale[bot] commented 1 year ago

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

stale[bot] commented 1 year ago

This pull request has been closed due to inactivity. If you feel this is in error, please reopen the pull request or file a new PR with the relevant details.