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

Inherit HTTPConnection through urllib3.connection, not httplib #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mupuf
Copy link

@mupuf mupuf commented May 5, 2023

By inheriting from urllib3.connection.HTTPConnection (that inherits from httplib.HTTPConnection itself), we can adapt to the internal changes in urllib3 2.0 that added a request() method that is incompatible with httplib.HTTPConnection.request.

This fixes the incompatibility between urllib3 2.0 and requests 1.26+, which was the first version that stopped vendoring urllib3.

Reference: docker/docker-py#3113 (comment)

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request May 10, 2023
…5688

https://build.opensuse.org/request/show/1085688
by user dgarcia + dimstar_suse
- Add urllib3-2.patch to make it compatible with urllib3 >= 2.0.0
  gh#msabramo/requests-unixsocket#69
@mgorny
Copy link

mgorny commented May 21, 2023

Thanks for figuring this out. I can confirm that with this patch, tests on both requests-unixsocket itself and on cheroot (using requests-unixsocket) pass. I'm going to carry it into Gentoo.

gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request May 21, 2023
Pull-Request: msabramo/requests-unixsocket#69
Signed-off-by: Michał Górny <mgorny@gentoo.org>
@webknjaz
Copy link

@mupuf could you add Fixes #70 somewhere in the PR description?

@webknjaz
Copy link

@msabramo this is a fix to a far-reaching issue. Would you mind merging it?

By inheriting from `urllib3.connection.HTTPConnection` (that inherits
from `httplib.HTTPConnection` itself), we can adapt to the internal
changes in urllib3 2.0 that added a `request()` method that is
incompatible with httplib.HTTPConnection.request.

This fixes the incompatibility between urllib3 2.0 and requests 1.26+,
which was the first version that stopped vendoring urllib3.

Reference: docker/docker-py#3113 (comment)
Fixes: msabramo#70
setup.cfg Outdated
@@ -1,5 +1,6 @@
[metadata]
name = requests-unixsocket
version = 0.2.1

Choose a reason for hiding this comment

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

Pretty sure this is wrong: the project version is produced from Git tags via pbr. Besides, the version on PyPI is 0.3.0 already.

Copy link
Author

Choose a reason for hiding this comment

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

Right, this was never intended to end up here. That's what I get for using master for a PR!

Copy link
Author

Choose a reason for hiding this comment

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

/me just wanted to make a release in his fork, until a new release was made upstream.

I also did not expect it would take so long to actually land.

Choose a reason for hiding this comment

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

Ah, makes sense. Have you checked if installing from Git URLs would work if you make a tag in your fork? Also, you could make a dumb-pypi index on GH Pages with a custom build and install from there.

FWIW I checked the maintainer's profile and it seems to be rarely used. So I wrote emailed them yesterday about this with suggestions. No reply so far.

@oschwartz10612
Copy link

Yes, @msabramo this is effecting our key applications. Can confirm the patch works in production. Merging would be appreciated.

webknjaz added a commit to cherrypy/cheroot that referenced this pull request Jul 2, 2023
This is needed because `requests-unixsocket` is currently incompatible
with the new release stream of `urllib3`[[1]] and it's unclear how
long it will take to get the bugfix PR[[2]] merged and a release
published.

[1]: msabramo/requests-unixsocket#70
[2]: msabramo/requests-unixsocket#69
@olemoign
Copy link

olemoign commented Sep 5, 2023

@msabramo Can you please merge this or give the rights to somebody to do it?

@thebaptiste
Copy link

@msabramo Can you please merge this or give the rights to somebody to do it?

Yes please, this a problem for me too. I would appreciate a merge and then a new release on pypi.

@thebaptiste
Copy link

@mupuf May be you could create a tag 0.3.1 or 0.4.0 (greater than release 0.3.0 on pypi) in your forked repo mupuf/requests-unixsocket ?
So I could use this tag with a git+https requirement on your repo. Of course, I could create my own fork, but as this fix is yours I would prefer to use your fork than mine !

@mupuf
Copy link
Author

mupuf commented Sep 12, 2023

@mupuf May be you could create a tag 0.3.1 or 0.4.0 (greater than release 0.3.0 on pypi) in your forked repo mupuf/requests-unixsocket ? So I could use this tag with a git+https requirement on your repo. Of course, I could create my own fork, but as this fix is yours I would prefer to use your fork than mine !

Already done in May: https://github.com/mupuf/requests-unixsocket/releases/tag/v0.2.1

@thebaptiste
Copy link

thebaptiste commented Sep 12, 2023

Already done in May: https://github.com/mupuf/requests-unixsocket/releases/tag/v0.2.1

Indeed, but 0.2.1 is lesser than 0.3.0 on pypi (even if this tag 0.3.0 should not be missing in git), so may be it's a little confusing to update from 0.3.0 (from pypi) to 0.2.1 (from git).
Thanks anyway for your fix, it works fine and allow me to use requests-unixsocket with the last releases of both requests and urllib3

@cquike
Copy link

cquike commented Oct 17, 2023

This is urgently needed by many projects who get errors by its users who pull the last version of requests-unixsocket from pypi. @msabramo : do you think there is something holding off the merging of this?
Thanks!

@erjoalgo
Copy link

+1 I am affected by this.

@erjoalgo
Copy link

erjoalgo commented Oct 30, 2023

I worked around this error using the following one-liner, which asks pip to install the fix from this branch:

pip install git+https://github.com/mupuf/requests-unixsocket --break-system-packages

Hopefully the fix will be merged soon.

@webknjaz
Copy link

Don't throw --break-system-packages around. Somebody is going to copy this and break their system one day. If one needs it for whatever reason, they'll add this option.

@webknjaz
Copy link

@mupuf how you feel about sticking your fork into some shared maintenance org and setting up publishing to PyPI under a different project name, like requests-unixsocket-urllib3-v2? Could move to one of the existing orgs or make a new one...

@mupuf
Copy link
Author

mupuf commented Nov 1, 2023

@webknjaz: I agree, it is time for someone to fork this project and become the new maintainer. This person won't however be me, so feel free to step up :)

@webknjaz
Copy link

webknjaz commented Nov 1, 2023

@mupuf I'm not talking about full maintenance, just publishing this patch and having a mechanism for letting other people start maintaining the thing. I also don't need to maintain it beyond making it installable from a more permanent location...

@oschwartz10612
Copy link

I have reached out to @msabramo over the email listed on his profile. I will let you know if I hear back. If needed, I can fork and setup publishing so we all can pull from a reliable source.

@crgwbr
Copy link

crgwbr commented Dec 28, 2023

Any update on getting this fix merged and released? Could we perhaps convince PSF (since they already control requests) or Jazzband to take over this project?

@webknjaz
Copy link

Jazzband won't accept it. I asked. There should still be a project leader and willingness to transfer.

@olemoign
Copy link

olemoign commented Jan 11, 2024

@kloczek
Copy link

kloczek commented Apr 6, 2024

it would be really good to merge this PR and release new version.

@crgwbr
Copy link

crgwbr commented May 22, 2024

Since this project seems to be abandoned, but its longevity is important to my team, we've forked the project as requests-unixsocket2. It should be a drop in replacement for this package.

We've migrated this PR there, merged it, and released to PyPI as part of v0.4.0.

@mgorny
Copy link

mgorny commented May 23, 2024

Since this project seems to be abandoned, but its longevity is important to my team, we've forked the project as requests-unixsocket2. It should be a drop in replacement for this package.

Are you planning to actively maintain the project going forward, or just keep it alive? If the former, why not open a PEP 541 request to take the original name? They take around forever to process, so the sooner the better.

@crgwbr
Copy link

crgwbr commented May 23, 2024

We're using it production on several actively developed projects, so I anticipate keeping it compatible with up-to-date requests/urllib3. But we're not at the moment looking to add a ton of new features/refactoring/etc, if that's what you mean. PRs are of course welcome if you have something in mind.

@simondeziel
Copy link

@crgwbr if you could take over the original name as suggested by @mgorny that'd ensure a smooth transition for everyone.

@cquike
Copy link

cquike commented Jul 2, 2024

Taking over the original name would probably also alleviate some transition pains for downstream. See for instance this Fedora bug report: https://bugzilla.redhat.com/show_bug.cgi?id=2284361

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.

None yet