Skip to content

Add HTTP/2 support and protocol version logging#232

Open
jguz-pubnub wants to merge 7 commits into
masterfrom
python-http2
Open

Add HTTP/2 support and protocol version logging#232
jguz-pubnub wants to merge 7 commits into
masterfrom
python-http2

Conversation

@jguz-pubnub
Copy link
Copy Markdown
Contributor

@jguz-pubnub jguz-pubnub commented May 14, 2026

feat: enable HTTP/2 negotiation on the synchronous httpx handler

feat: add http_version field to ResponseInfo

feat: log negotiated protocol version at DEBUG level across all request handlers

- feat: enable HTTP/2 negotiation on the synchronous httpx handler
- feat: add http_version field to ResponseInfo
- feat: log negotiated protocol version at DEBUG level across all request handlers
@jguz-pubnub jguz-pubnub requested a review from parfeon as a code owner May 14, 2026 10:29
@pubnub-ops-terraform
Copy link
Copy Markdown

pubnub-ops-terraform commented May 14, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment on lines +177 to +180
logger.debug(
"PubNub request completed: operation=%s protocol=%s"
% (options.operation_type, response_info.http_version)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

debug probably over informative, but we don't have other levels yet.
I would suggest to merge it with data log above.

Comment thread pubnub/request_handlers/async_httpx.py Outdated
Comment on lines +229 to +232
logger.debug(
"PubNub request completed: operation=%s protocol=%s"
% (options.operation_type, response_info.http_version)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

debug probably over informative, but we don't have other levels yet.
I would suggest to merge it with data log above.

Comment thread pubnub/request_handlers/requests.py Outdated
Comment on lines +281 to +284
logger.debug(
"PubNub request completed: operation=%s protocol=%s"
% (e_options.operation_type, http_ver)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we actually print it if version information available in response?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here's what I've been told to deliver:

Deliverables:

Debug logs such as:

PubNub request completed: operation=subscribe protocol=HTTP/2
PubNub request completed: operation=publish protocol=HTTP/1.1

I'll try to make it less noisy and merge it with data logs you suggested

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, I've missed operation in the format.

Comment thread requirements-dev.txt
Comment thread pubnub/request_handlers/httpx.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one should be instructed in a same way as AsyncClient need to specify http2=True in constructor.

Comment thread pubnub/request_handlers/async_httpx.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But where is http2=True in constructor?

Comment thread pubnub/request_handlers/async_aiohttp.py
Comment thread pubnub/request_handlers/requests.py
Copy link
Copy Markdown
Contributor

@parfeon parfeon left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

3 participants