Fix HTTP/2 keep-alive channel options - #268
Merged
Merged
Conversation
github-actions
Bot
added
part:docs
Marenz
force-pushed
the
add-dependabot-workflow
branch
from
August 11, 2026 14:29
7dd163e to
f481515
Compare
Marenz
marked this pull request as ready for review
August 11, 2026 14:31
There was a problem hiding this comment.
Pull request overview
This PR fixes gRPC channel construction so HTTP/2 keep-alive is actually applied (by ensuring channel args are passed as int milliseconds) and adds support for connecting to an explicit IPv4/IPv6 socket address while preserving the original URI hostname for HTTP/2 authority and TLS verification.
Changes:
- Convert keep-alive interval/timeout to integer milliseconds to avoid grpc-python silently ignoring
floatchannel args. - Introduce
ChannelOptions.connection_addressto connect to a specific IP while retaining the URI hostname via gRPC authority/TLS override options. - Extend tests to cover address overrides and to assert keep-alive arg types.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/frequenz/client/base/channel.py |
Adds connection-address override support and ensures keep-alive options are emitted as int milliseconds. |
tests/test_channel.py |
Adds regression coverage for keep-alive argument types and connection-address override behavior/validation. |
RELEASE_NOTES.md |
Documents the keep-alive fix and the new connection-address override feature. |
AGENTS.md |
Adds repository agent guidance (build/test commands, conventions, architecture notes). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Marenz
force-pushed
the
add-dependabot-workflow
branch
from
August 11, 2026 14:39
f481515 to
6d1af36
Compare
llucax
reviewed
Aug 11, 2026
llucax
reviewed
Aug 11, 2026
Marenz
changed the title
`grpc.keepalive_time_ms` and `grpc.keepalive_timeout_ms` were computed with `timedelta.total_seconds() * 1000`, which yields a `float`. grpc-python silently ignores channel arguments whose value is neither `int` nor `str`, so keep-alive was never armed: no pings were sent and a client could only notice a dead stream through its own deadline or a teardown from the peer. Convert both durations to whole milliseconds via a small `_to_millis` helper. The regression test asserts the exact argument type, because value equality does not distinguish `60000` from `60000.0`. Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
Marenz
force-pushed
the
add-dependabot-workflow
branch
from
August 12, 2026 12:04
90c205d to
0e66abc
Compare
Marenz
changed the title
llucax
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
grpc.keepalive_time_msandgrpc.keepalive_timeout_mswere passed asfloat.grpc-python silently ignores channel arguments that are neither
intnorstr,so keep-alive had never actually been armed.
The regression test asserts the exact argument types, because value equality alone
does not distinguish
60000from60000.0.Verified with grpc-python keepalive traces and TCP
lastsnd: the patched channelemitted pings at the configured interval. Keep-alive only proves that the nearest
HTTP/2 peer is alive, so terminating proxies need their own backend liveness handling.
Checks:
uv run nox -R -s formatting flake8 mypy pylint pytest_max(133 tests passed).