python / typeshed

Collection of library stubs for Python, with static types
Other
4.39k stars 1.77k forks source link

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

Closed jessemyers-lettuce closed 1 year ago

jessemyers-lettuce commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

grutts commented 1 year ago

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 commented 1 year ago

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.

srittau commented 1 year ago

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.

aquamatthias commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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:

setu4993 commented 1 year ago

@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 https://github.com/python/typeshed/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/python/typeshed/pull/10839 to add a note to the main PyPI description as well.

AlexWaygood commented 1 year ago
  • 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 https://github.com/typeshed-internal/stub_uploader/issues/104 initially, but the discussion is now continuing at https://github.com/python/typeshed/issues/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 commented 1 year ago
  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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/python/typeshed/pull/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 (https://github.com/boto/botocore/pull/3034).

AlexWaygood commented 1 year ago

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: https://github.com/typeshed-internal/stub_uploader/commit/1feee283b1f3881468536606d83af761481be7b3

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.)