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

types-requests>=2.31.0.7 can't be installed alongside urllib3<2 #10825

Closed
jessemyers-lettuce opened this issue Oct 2, 2023 · 25 comments
Closed

Comments

@jessemyers-lettuce
Copy link

This PR creates a breaking change because it adds:

urllib3>=2

As of this writing, requests itself has this dependency declaration:

urllib3>=1.21.1,<3

This means that there are scenarios where requests can be installed, but types-requests cannot.

In my case, I am using boto3 and botocore for AWS interactions and it defines its own urllib3 dependency range that produces this incompatiblity:

urllib3>=1.25.4,<1.27

Please consider aligning the types-requests version ranges to the same bounds that requests itself uses.

@AlexWaygood
Copy link
Member

If you have a requirements file that also includes a package that pins urllib3>=1.25.4,<1.27, pip (or any other standards-based installer) should be able to determine that the maximum version of types-requests that can be installed in types-requests==2.31.0.6, since the latest version depends on urllib3>=2. If that doesn't work for some reason, would pinning types-requests to <2.31.0.7 be an option for you?

@AlexWaygood
Copy link
Member

urllib3<2 will unfortunately not work for our types-requests stubs, as they have to have a version of urllib3 with typing information.

@jessemyers-lettuce
Copy link
Author

@AlexWaygood My goal is always to use the most recent version of every dependency that I can without breaking my systems. I use an automated process (renovate) to continuously integrate the latest.

So yes, I am pinning a specific version.

But... it seems to me that you've taken a step to optimize your development process -- I don't know what these stubs do so I can't comment on the value added -- and this comes at the expense of the users of your library.

Put another way: it seems obvious to me that the latest version of types-requests should be 100% compatible with the latest version of requests OR types-requests should have an upper bound version for requests itself (which is probably not something you want).

@AlexWaygood
Copy link
Member

But... it seems to me that you've taken a step to optimize your development process -- I don't know what these stubs do so I can't comment on the value added -- and this comes at the expense of the users of your library.

To be clear, without this change, it was painful for our users to use types-requests alongside the latest version of urllib3 (see #10786). This change resolved that problem. It's hard for me to see a way that we could solve that problem and your problem simultaneously here.

@jessemyers-lettuce
Copy link
Author

It seems like there are two obvious alternatives:

  1. Figure out how to get widely used libraries (e.g. botocore) to use newer versions of requests; this isn't my choice and I am sure that there are a large number of users out there in a similar boat.
  2. Write your types to detect the version of urllib3 and fallback to what you had before for older versions.

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 2, 2023

2. Write your types to detect the version of urllib3 and fallback to what you had before for older versions.

Stubs are never executed at runtime. In a runtime package you might do something like this to resolve this problem...

try:
    import urllib3
except ImportError:
    urllib3 = None
else:
    if int(urllib3.__version__.split(".")[0]) < 2:
        urllib3 = None

if urllib3 is None:
    import types_urllib3 as urllib3

...but that's out of the question for a stubs package, unfortunately, since a stubs package entirely consists of "data files" that are to be read by a static type checker. All of that stuff is way too dynamic for a type checker to understand; types-requests really has to depend on either urllib3>=2 or types-urllib3 (which is a stubs package for urllib3<2). I don't see a way of falling back to types-urllib3 if urllib3>=2 isn't installed.

@jessemyers-lettuce
Copy link
Author

It seems like your implementation choices are getting in the way. I'm going to assume that these choices aren't up for debate at this time.

A few options to consider:

  1. Don't make this kind of change without some sort of major version indicator.
  2. Offer the old behavior under a different dependency name (e.g. types-requests-old-stubs); this would allow a user to more obviously declare a dependency without an auto-upgrade path introducing breakages.
  3. Document the expected usage. I'm not the only person who will experience this issue.

@setu4993
Copy link

setu4993 commented Oct 2, 2023

Just ran into this change and I do agree with the @jessemyers-lettuce above on:

it seems obvious to me that the latest version of types-requests should be 100% compatible with the latest version of requests OR types-requests should have an upper bound version for requests itself (which is probably not something you want).

If that's not the expected behavior, I wouldn't expect such a major change to be introduced as a part of a patch to a patch (v2.31.0.7). This should at the very least, be a minor version update, and ideally a breaking change.

@AlexWaygood AlexWaygood changed the title types-requests introduces urllib3 incompatibility types-requests can't be installed alongside urllib3<2 Oct 3, 2023
@AlexWaygood AlexWaygood changed the title types-requests can't be installed alongside urllib3<2 types-requests>=2.31.0.7 can't be installed alongside urllib3<2 Oct 3, 2023
@grutts
Copy link

grutts commented Oct 3, 2023

We are also encountering an issue with this change. Semgrep does not support urllib3 > 1.26 in their latest version. The ensuing pip conflict means we are unable to build a number of services which use both semgrep and types-requests. We will pin types-requests to an older version for the time being.

@hauntsaninja
Copy link
Collaborator

Thanks everyone for chiming in!

Stub version numbers primarily reflect the version of the upstream package that they represent. This is important as it lets users more clearly type check their code based on the versions of upstream packages they're using.

Therefore, changes to stubs that are not retargeting to a different upstream version end up changing later digits in the version (also basically every change to a stub can change type checker behaviour, so hard to ascribe too much semantic meaning to them)

I agree it would have been nice if we'd had the foresight to bundle this change in with a version bump in upstream requests, but as it stands, there's not too much to be done. Dependency resolvers should be able to handle this and figure out a compatible set of packages just fine. You're welcome to manually pin to older types-requests as well.

@AlexWaygood AlexWaygood changed the title types-requests>=2.31.0.7 can't be installed alongside urllib3<2 types-requests>=2.31.0.7 can't be installed alongside urllib3<2 Oct 3, 2023
@srittau
Copy link
Collaborator

srittau commented Oct 4, 2023

I agree with @AlexWaygood and @hauntsaninja here. There is not much that can be done on typeshed's side. The underlying issue is that boto depends on an old version of urllib3. (The underlying underlying issue is the flat namespace of Python imports, which is not something that will change in the short run or ever.) While I understand that it's frustrating to deal with this issue, this is worked around by using proper dependency management (which should prevent newer versions of types-requests to be installed) and/or pinning the types-requests dependency.

@srittau srittau closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2023
@aquamatthias
Copy link

I have read the comments. While I see all the challenges, closing this issue is unsatisfactory.

My setup:

  1. I use poetry to pin all versions of all my libraries: poetry install
  2. I run mypy to check my code like this: mypy --install-types --non-interactive ...

Installing the types via mypy will have this effect:

Installing collected packages: types-requests
  Attempting uninstall: urllib3
    Found existing installation: urllib3 1.26.17
    Uninstalling urllib3-1.26.17:
      Successfully uninstalled urllib3-1.26.17
Successfully installed types-requests-2.31.0.7 urllib3-2.0.6

Installing type information now has an effect on the installed libraries and breaks the setup.
You could blame it on mypy, but honestly I think it is a problem of the dependency chain.

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 4, 2023

@aquamatthias that does sound unfortunate. I'm sorry for the disruption we've caused to your workflow here.

FWIW, I'd strongly recommend against using --install-types with mypy, as it's extremely slow. Mypy has to run twice over your code: once to figure out which stubs it needs to install, then (after installing those stubs) a second time to figure out which (if any) typing errors are still present after the stubs have been installed. This is mentioned in the mypy docs, though the option could perhaps be more actively discouraged: https://mypy.readthedocs.io/en/stable/running_mypy.html#library-stubs-not-installed. Note also that the risk of this kind of thing happening is why --non-interactive is not the default for --install-types.

Since you're using poetry, have you considered adding a dev-dependency group of optional requirements, to list the stubs packages that are required to type-check your project with mypy?

@aquamatthias
Copy link

Since you're using poetry, have you considered adding a dev-dependency group of optional requirements, to list the stubs packages that are required to type-check your project with mypy?

@AlexWaygood Yes - that is our solution for types-requests. We probably should define all of them as suggested.
Thanks for pointing it out.

@jessemyers-lettuce
Copy link
Author

jessemyers-lettuce commented Oct 5, 2023

Let me start by saying that I understand that there are limits in terms of what this project can do. That said, I worry about two things in particular:

  • First, there seems to be an assumption within typeshed that versions operate differently than they do elsewhere (e.g. that versions track with the upstream project and don't define their own scheme). But none of the obvious documentation (e.g. this project's README or linked docs) documents this to be the case. It does not seem likely that any outside developer will understand the expectation here and that seems like a gap. There also seems to be an assumption that developers using typeshed will be using specific packaging tools or approaches and this seems even less likely to be known (or accepted) to developers en masse. Even if I don't agree with these assumptions, it would be invaluable to have them be stated explicitly so future discussions start from the same context.

  • Second, there seems to be a possible/hypothetical short-coming of the "use foo package manager" or "pin bar version" answer here. There's nothing stopping a future version of an upstream library (e.g. requests) from introducing typing that cannot be used without also upgrading types-requests/types-urllib3 to match. And if the solution to the problem here is to pin dependencies, the types library would effectively be holding back developers from using such a new version. I freely admit this scenario is not the one we have right now, but it feels plausible and is part of why I would assume -- at least prior to this thread -- that a types installation should be fully compatible with its upstream's dependencies.

@setu4993
Copy link

setu4993 commented Oct 5, 2023

@AlexWaygood @hauntsaninja @srittau : Thanks for engaging and responding. Appreciate the work you'll are doing in maintaining the stubs.

The problem here, IMHO, is two-fold:

  1. This package, types-requests provides stubs for requests but is now incompatible with the versions of urllib3 that requests supports (upstream source. This, to me, is fundamentally incorrect. The dependencies for type-stubs should not be supported on a subset of versions of the original package. That reduces the value of the stubs because they cannot be applied to the package and are only useful for a subset! For that reason, I'd argue that the resolution for Installing types-requests breaks urllib3 2.0 type hints #10786 by dropping support for a fair chunk of urllib3 is premature (and incorrect). Accommodating this change in our projects for now works, but what about when requests v2.32 comes out? Why should our stubs be stuck on a backward version, even though requests continues to support urllib3 v1?

Stub version numbers primarily reflect the version of the upstream package that they represent. This is important as it lets users more clearly type check their code based on the versions of upstream packages they're using.

  1. This expectation is exactly why it is absurd to me that a change to drop support for a major version of a transient dependency occurred in a micro-version. Alongside point 1 above, this means that v2.31.0.7+ are now incompatible with requests, which goes against the expectation outlined by you. Sure, the version number here matches but there's a slimmer set of environments in which these both packages can now co-exist than did with v2.31.0.6.

Thank you for suggesting the workarounds, and we have (as I'm sure others have) already implemented them in their workflows. That doesn't change the fact that this is an issue worth fixing and has been hurriedly closed without regard for fair points and concerns.

@srittau
Copy link
Collaborator

srittau commented Oct 5, 2023

Typeshed versions work the way they do due to technical limitations. Other schemes were debated, but had significant drawbacks. That said, this discussion showed that we need to communicate our versioning policy better and we could possibly iterate on the exact way we use versions. I've opened #10837 to further discuss this.

@calpaterson
Copy link
Contributor

calpaterson commented Oct 5, 2023

Dependency resolvers should be able to handle this and figure out a compatible set of packages just fine

this is worked around by using proper dependency management

I think I agree with the approach types-requests has taken but I think it's important to note that in many cases pip is not actually managing to find solutions and is unable to complete operations. People are here because their builds have failed.

Perhaps it would be useful to add a note somewhere prominent that users should pin your lib as types-requests<2.31.0.7 if they run into problems with the communities' incomplete adoption of urllib3 v2?

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 5, 2023

Perhaps it would be useful to add a note somewhere prominent that users should pin your lib as types-requests<2.31.0.7 if they run into problems with the communities' incomplete adoption of urllib3 v2?

@calpaterson I tried to flag this as loudly as I could in the CHANGELOG entry, which is linked to from the PyPI page for types-requests: https://github.com/typeshed-internal/stub_uploader/blob/main/data/changelogs/requests.md#23107-2023-10-01. But, I think it's fair to say that this change has been more disruptive than I anticipated :) I've filed #10839 to add a note to the main PyPI description as well.

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 5, 2023

  • First, there seems to be an assumption within typeshed that versions operate differently than they do elsewhere (e.g. that versions track with the upstream project and don't define their own scheme). But none of the obvious documentation (e.g. this project's README or linked docs) documents this to be the case. It does not seem likely that any outside developer will understand the expectation here and that seems like a gap. There also seems to be an assumption that developers using typeshed will be using specific packaging tools or approaches and this seems even less likely to be known (or accepted) to developers en masse. Even if I don't agree with these assumptions, it would be invaluable to have them be stated explicitly so future discussions start from the same context.

@jessemyers-lettuce I think you're raising very reasonable points about our inadequate documentation here around our versioning scheme. I myself don't fully understand all the ins and outs of it. We need to do better here. Whether or not we change our versioning scheme (discussed at typeshed-internal/stub_uploader#104 initially, but the discussion is now continuing at #10837), we have to do better at communicating it. I'm going to try to write up some better documentation for us at some point in the next few days.

(EDIT: @srittau beat me to it! #10840 has now been filed to improve our documentation here.)

@AlexWaygood
Copy link
Member

  1. This package, types-requests provides stubs for requests but is now incompatible with the versions of urllib3 that requests supports (upstream source. This, to me, is fundamentally incorrect.

@setu4993, to be clear, this was already the case prior to the change that happened here. Prior to this change, types-requests was incompatible with urllib3>=2 (and note that urllib3 released version 2 six months ago). See #10786, which was filed by a urllib3 maintainer about this exact issue. Prior to the change we're discussing here, types-requests depended on types-urllib3, which was a stubs package for urllib3<2. If mypy finds types-urllib3 and urllib3>2 in the same environment, it ignores all type annotations in urllib3>2, preferring types-urllib3 instead. (This is the correct behaviour according to PEP-561.) You can understand, I'm sure, why it would be extremely annoying for users of urllib3>=2 for mypy to completely ignore the up-to-date inline annotations in urllib3>=2 in favour of out-of-date annotations in types-urllib3, just because they've installed types-requests. So types-requests was already incompatible with some versions of urllib3 that requests supports -- and I don't think there's really any way around that.

What's changed is three things:

  1. The latest version of our stubs is now explicit about being incompatible with certain versions of urllib3, rather than being implicitly incompatible with certain versions of urllib3, as was previously the case.
  2. Prior to this change, there were versions of types-requests that users of urllib3<2 could use, but there were no versions of types-requests that users of urllib3>=2 could use. Now, there are versions of types-requests that both sets of users can use: users of urllib3<2 can pin to types-requests<2.31.0.7, and users of urllib3>=2 can pin to types-requests>=2.31.07.
  3. Prior to this change, the latest version of types-requests was incompatible with urllib3>=2; now, the latest version of types-requests is incompatible with urllib3<2.

@jessemyers-lettuce
Copy link
Author

@AlexWaygood Thanks for the detailed and thoughtful response and I'm glad that versioning will be communicated more clearly.

re:

to be clear, this was already the case prior to the change that happened here

I think it's important to recognize that there are two different kinds of incompatibility here and that most people care first about runtime compatibility and second about type definition compatibility. The move towards explicitness re: type definitions now has an impact on runtime, which is why everyone is surprised.

@AlexWaygood
Copy link
Member

I think it's important to recognize that there are two different kinds of incompatibility here and that most people care first about runtime compatibility and second about type definition compatibility. The move towards explicitness re: type definitions now has an impact on runtime, which is why everyone is surprised.

Yep, I get that, and I'd like to apologise again for not anticipating in advance how disruptive/unexpected this change would be.

The idea of having typeshed stubs that depend on external runtime packages is still ~pretty new (#5768 was only closed in January), so to some extent we're still figuring out this brave new world. I think we were probably focussed too much on the many alterations that were needed to our testing infrastructure to make that change, and didn't think enough about communicating the full implications of that change to our users.

@setu4993
Copy link

setu4993 commented Oct 7, 2023

Thanks for engaging constructively and in good spirit through this discussion.

to be clear, this was already the case prior to the change that happened here

While this is correct, the mismatch IMHO lies in the fact that prior to this change urllib3>2 couldn't use type definitions whereas now urllib3<2 cannot use the latest / fixed type definitions.

Maybe just me, but incorrectly validated / missing type definitions is better than incorrect type definitions which don't match against the latest upstream package / fixes.

@jessemyers-lettuce correctly summarized the distinction as:

type definitions now has an impact on runtime, which is why everyone is surprised.


I do think better documentation as in #10840 (thank you!) do help atleast raise awareness about what expectations users should have from type definitions.

This has also been an (or atleast one of) impetus to push for other packages to adopt urllib3>2 (botocore, in our case), which has now added support for urllib3>2 (boto/botocore#3034).

@AlexWaygood
Copy link
Member

I do think better documentation as in #10840 (thank you!) do help atleast raise awareness about what expectations users should have from type definitions.

FWIW, we've also been trying to improve the generated PyPI descriptions for these stubs packages: typeshed-internal/stub_uploader@1feee28

Here's an example of what the new-style descriptions look like: https://pypi.org/project/types-Markdown/3.5.0.0/

(It'll take a while for this to filter through into the PyPI descriptions for all our existing stubs packages; they'll only get new PyPI releases as and when they get updates at typeshed. types-Markdown had a typeshed update the other day, which means it's already got the new-style PyPI description.)

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

No branches or pull requests

8 participants