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

Tracking breaking changes for v2.0 #2065

Closed
26 of 27 tasks
pquentin opened this issue Nov 16, 2020 · 12 comments · Fixed by #2361
Closed
26 of 27 tasks

Tracking breaking changes for v2.0 #2065

pquentin opened this issue Nov 16, 2020 · 12 comments · Fixed by #2361
Assignees
Milestone

Comments

@pquentin
Copy link
Member

pquentin commented Nov 16, 2020

We need to record all noteworthy 2.0 changes as news fragments. Here's the full list: 1.26.x...main. I've listed all relevant changes below. Sorry, this is messy, I sometimes link to the merged commit, and sometimes to the PR. The merged commit is usually a better starting point as Seth usually rewords before merging.

Original issue below.


We just removed two bits of API in 2.x because they're no longer useful in Python 3:

Should we:

  1. just put that in the changelog and be done with it?
  2. or make AppEngineManager an alias of PoolManager and accept but ignore strict?

I think we said we were aiming for zero API breakage of that sort, and @untitaker has mentioned it would help the Sentry SDK. I'd be in favor of option 2.

@pquentin pquentin changed the title How should we handle removed API in 2.x? [v2] How should we handle removed API in 2.x? Nov 16, 2020
@sethmlarson
Copy link
Member

  • AppEngineManager is good to remove since in theory it's impossible to install urllib3 v2.0 on Python 2.7 and that fixture is only usable from Python 2.7
  • strict is different, because code may have been passing strict=True just for the effects on Python 2.7 we should probably just pop it out of **kwargs like we were doing before. I'm not sure there's even value in emitting a warning in this case, we can likely just exclude it from type hints to push users away from it. If we want to be more pushy we can emit a DeprecationWarning but then it becomes a pain for packages supporting Python 2 still.

@sethmlarson
Copy link
Member

Unfortunately we'll likely have to handle them on a case-by-case basis. Potentially we can keep this issue open so we can remember how we handled each and that'll help with writing the migration guide.

@pquentin
Copy link
Member Author

Another breaking change that should be mentioned in the changelog is getheaders() return signature changing.

@sethmlarson
Copy link
Member

@sethmlarson
Copy link
Member

Breaking change: Removed urllib3.util.current_time, use time.monotonic instead (#2214)

@sethmlarson sethmlarson changed the title [v2] How should we handle removed API in 2.x? Tracking breaking changes for v2.0 Jun 10, 2021
@sethmlarson sethmlarson added this to the v2.0 milestone Jun 10, 2021
@sethmlarson sethmlarson pinned this issue Jun 10, 2021
@sethmlarson
Copy link
Member

Multipart header parameters now formatted to modern WHATWG HTML standard: #2257

@pquentin
Copy link
Member Author

pquentin commented Aug 3, 2021

We now use towncrier, so I'm going to repurpose that issue to make sure we record all noteworthy v2 changes as news fragments. That way, we won't have to track every change down when we'll actually release 2.0.

@sethmlarson
Copy link
Member

@pquentin Agreed, would you be willing to tackle that?

@pquentin
Copy link
Member Author

pquentin commented Aug 3, 2021

Yes, I'm on it! I have 167 commits to review.

@pquentin
Copy link
Member Author

pquentin commented Aug 3, 2021

I've added 27 potential news fragments to the first comment. Some of them are already in the news fragments, and some of them may turn out to be irrelevant, but that's easier to work with than the original 167 commits :)

@pquentin
Copy link
Member Author

pquentin commented Aug 4, 2021

Should we turn this into a good first issue? Anyone can add a few news fragments, once merged we can mark them as done above. See https://github.com/urllib3/urllib3/tree/main/changelog for the related documentation.

Or is this something that we should do ourselves?

@sethmlarson
Copy link
Member

@pquentin We should probably do this ourselves, otherwise it'll be a lot for us to manually review anyways.

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 a pull request may close this issue.

2 participants