Feature: optional pycurl #2269

spawn-guy · 2025-03-24T14:39:59Z

as it was reported urllib3 might be slow(er) than pycurl as an http client. (or even "not working" in 1 unverified report)

this PR brings the curl back. and uses pycurl when available, reverting to urllib3 when not

as the pycurl dependency was removed from being required by sqs extra module
to use pycurl - users need to explicitly add and install pycurl library on their own.

the last required version in pip/requirements.txt format was

pycurl>=7.43.0.5; sys_platform != 'win32' and platform_python_implementation=="CPython"

codecov · 2025-03-24T14:44:31Z

Codecov Report

Attention: Patch coverage is 16.00000% with 168 lines in your changes missing coverage. Please review.

Project coverage is 80.20%. Comparing base (f224000) to head (5b40be2).

Files with missing lines	Patch %	Lines
kombu/asynchronous/http/curl.py	15.73%	162 Missing and 4 partials ⚠️
kombu/asynchronous/http/__init__.py	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2269      +/-   ##
==========================================
- Coverage   81.55%   80.20%   -1.35%     
==========================================
  Files          77       78       +1     
  Lines        9541     9741     +200     
  Branches     1162     1199      +37     
==========================================
+ Hits         7781     7813      +32     
- Misses       1568     1731     +163     
- Partials      192      197       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Nusnus

The py-kafka failures are not related

auvipy · 2025-03-25T07:51:37Z

v5.7 or 5.6 for this?

auvipy · 2025-03-25T07:56:38Z

kombu/asynchronous/http/__init__.py

@@ -9,6 +9,10 @@

 def Client(hub: Hub | None = None, **kwargs: int) -> BaseClient:
    """Create new HTTP client."""


will this work as expected? we are not using any try except block here, I'm open to better suggestions as well

i've looked at try-catch, but liner was complaining. and.. the .curl code already has this ImportError exception handling. so until the CurlClient is instantiated - there will be no exception. however the CurlClient.Curl will be None.

ima gonna test the code now

the code seems to work without pycurl and no thrown Exceptions

auvipy · 2025-03-25T08:23:21Z

hopefully the kafka builds will pass now

auvipy · 2025-03-26T06:14:44Z

kafka is passing now

auvipy · 2025-03-27T05:44:57Z

for a saner approach, should we just revert it first and then you come with your urllib3 patch again cleanly but keeping pycurl as default one?

spawn-guy · 2025-03-27T11:03:22Z

@auvipy the reverting is not great for me.
i now consider pycurl to be a part of "speedup" extra

but the reported problem is in the existing code from before my changes

Nusnus · 2025-03-27T13:45:56Z

@spawn-guy

but the reported problem is in the existing code from before my changes

So the slow downs aren't related to the dep changes?
Very interesting

spawn-guy · 2025-03-29T06:38:13Z

@spawn-guy

but the reported problem is in the existing code from before my changes

So the slow downs aren't related to the dep changes?
Very interesting

I can't yet figure out how to install pycurl on awslinux2023 yet (this is why I picked this migration in the first place) to test both speeds.
But I was posting inconsistent speedtest results for some time now

auvipy · 2025-03-29T07:15:25Z

@spawn-guy

but the reported problem is in the existing code from before my changes

So the slow downs aren't related to the dep changes? Very interesting

no they are somewhat related. also we had to use sqs internal / private api which was faster, later we moved to saner public api but with performance regression, then again revert and adding new code. and recently moving to new json protocol properly.

auvipy · 2025-03-29T07:17:08Z

kombu/asynchronous/http/__init__.py

@@ -9,6 +9,10 @@

 def Client(hub: Hub | None = None, **kwargs: int) -> BaseClient:
    """Create new HTTP client."""


auvipy · 2025-03-29T07:19:06Z

requirements/pkgutils.txt

@@ -6,3 +6,4 @@ sphinx2rst>=1.0
 bumpversion==0.6.0
 pydocstyle==6.3.0
 mypy==1.14.1
+types-pycurl>=7.43.0.5; sys_platform != 'win32' and platform_python_implementation=="CPython"


what is types-pycurl? did the package name changed?

Was suggested by mypy. Otherwise mypy ci was failing

Weren't we using only pycurl?

I started to get this error only now, and types-pycurl was suggested. The error was related to mypy build (which I am not very familiar with). Then I saw other types-* are already used from the same repo. And then if pycurl is optional then full pycurl dependency is not required and types-pycurl would be sufficient to pass mypy build. I suppose mypy build does typechecking that is why it errored

spawn-guy · 2025-03-31T13:48:12Z

just tested the code. finally solved my pycurl installation automation on amzn2023

so the code in this PR - WORKS! both when pycurl is and is not installed.
the reported str-to-bytes error is also fixed.

however,.. i'll post speed tests soon back in the issue ticket #2258

Nusnus

It appears to be working finally, thank you so much @spawn-guy 🙏

I'll review it this week

This reverts commit ea86b85.

…llib3 when not

for more information, see https://pre-commit.ci

spawn-guy mentioned this pull request Mar 24, 2025

Update to Kombu 5.5.0 stopped processing SQS messages #2258

Open

Nusnus added this to the 5.7.0 milestone Mar 24, 2025

Nusnus requested review from auvipy and Nusnus March 24, 2025 19:11

Nusnus reviewed Mar 24, 2025

View reviewed changes

auvipy requested changes Mar 25, 2025

View reviewed changes

auvipy modified the milestones: 5.7.0, 5.6.0 Mar 25, 2025

auvipy requested changes Mar 29, 2025

View reviewed changes

Nusnus requested changes Mar 31, 2025

View reviewed changes

spawn-guy and others added 8 commits April 7, 2025 11:08

Revert "style(pycurl): remove curl-related code"

c4cce86

This reverts commit ea86b85.

feat(pycurl): bring the curl back

7764c89

feat(pycurl): try importing pycurl when available and fall back to ur…

d547219

…llib3 when not

docs: add self-attribution

5f79c1b

[pre-commit.ci] auto fixes from pre-commit.com hooks

fcd46de

for more information, see https://pre-commit.ci

ci: fixing lint errors

2f0359a

ci: fix unused pycurl lint error

aabaa64

fix(curl): fix body bytes encoding

5b40be2

spawn-guy force-pushed the feature_optinal_pycurl branch from 1e9a732 to 5b40be2 Compare April 7, 2025 09:09

		@@ -9,6 +9,10 @@

		def Client(hub: Hub \| None = None, **kwargs: int) -> BaseClient:
		"""Create new HTTP client."""

Uh oh!

Feature: optional pycurl #2269

Are you sure you want to change the base?

Feature: optional pycurl #2269

Uh oh!

Conversation

spawn-guy commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Nusnus left a comment

Choose a reason for hiding this comment

Uh oh!

auvipy commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spawn-guy Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

auvipy commented Mar 25, 2025

Uh oh!

auvipy commented Mar 26, 2025

Uh oh!

auvipy commented Mar 27, 2025

Uh oh!

spawn-guy commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nusnus commented Mar 27, 2025

Uh oh!

spawn-guy commented Mar 29, 2025

Uh oh!

auvipy commented Mar 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spawn-guy commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nusnus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spawn-guy commented Mar 24, 2025 •

edited

Loading

codecov bot commented Mar 24, 2025 •

edited

Loading

spawn-guy Mar 25, 2025 •

edited

Loading

spawn-guy commented Mar 27, 2025 •

edited

Loading

spawn-guy commented Mar 31, 2025 •

edited

Loading