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

mbeliaev/setup_fix #691

Closed
wants to merge 1 commit into from
Closed

mbeliaev/setup_fix #691

wants to merge 1 commit into from

Conversation

beliaev-maksim
Copy link

need to fix the version due to change in cryptography release. See pyca/cryptography#6344 and https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst

@@ -44,7 +44,7 @@ docs =
sphinx-rtd-theme
zope.interface
crypto =
cryptography>=3.3.1,<4.0.0
cryptography>=3.3.1,<50.0.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cryptography>=3.3.1,<50.0.0
cryptography>=3.3.1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose initially there was some idea by limiting the version. I personally also do not see a need to set upper boundary.
I would wait for repo owners to confirm the preferred way

Copy link
Contributor

@rayluo rayluo Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use the cryptography (X+3).0.0 as the upper bound, i.e. <38.0.0 for today, based on their latest deprecation policy https://cryptography.io/en/latest/api-stability/#deprecation

Wait for repo owners to confirm the preferred way.

Copy link

@graingert graingert Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recommendation is not to set an upper bound:

Over the 7 years and > 600 million downloads we haven't seen our policies to be a problem in the real world, so our advice is ultimately: don't pin your upper bound. Users can receive security updates, a critical thing for a cryptographic library, and the likelihood of breakage is low.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recommendation is not to set an upper bound:

Over the 7 years and > 600 million downloads we haven't seen our policies to be a problem in the real world, so our advice is ultimately: don't pin your upper bound. Users can receive security updates, a critical thing for a cryptographic library, and the likelihood of breakage is low.

Yeah, that was a conversation that I started and I commented. As a mid-tier library maintainer, we would need to factor in the situation that a behavior we used today, being gone at version X+3.0.0 in the future. It is a trade-off that each repo's owner needs to make.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpadilla
how do you look if we remove upper boundary?

basically now major versions will change and setting any upper limit will not have a strong effect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically now major versions will change

True

and setting any upper limit will not have a strong effect

Not necessarily. It depends.

  • Setting any irrelevant upper limit - or setting no upper limit - would risk our package break at cryptography X+3 version.
  • Setting upper limit as <X+3.0.0 would make sure our current package always work, because it will never install a potentially incompatible dependency in future. This approach implies that we the maintainers would need to keep an eye on upstream cryptography's major version release, and then test and/or review it, then bump our upper bound by 1. This would happen once every several months.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the advice from @reaperhulk is clear:

our advice is ultimately: don't pin your upper bound.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants