Skip to content

feat(python): expose TCP client configuration - #3776

Open
ethanlin01x wants to merge 29 commits into
apache:masterfrom
ethanlin01x:feat/python-tcp-config
Open

feat(python): expose TCP client configuration#3776
ethanlin01x wants to merge 29 commits into
apache:masterfrom
ethanlin01x:feat/python-tcp-config

Conversation

@ethanlin01x

@ethanlin01x ethanlin01x commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Closes #3742

Rationale

The Python binding accepts only a bare server address, so reconnection and auto-login cannot be configured from Python. The SDK's session recovery is therefore unreachable: a server restart surfaces as Unauthenticated on the next call.

What changed?

IggyClient(...) took only host:port, with AutoLogin::Disabled hardcoded and the reconnection policy untunable.

It now also accepts a keyword-only TcpConfig mirroring the Rust TcpClientConfig (auto_login, reconnection, heartbeat_interval, TLS, nodelay). Unset fields fall back to the Rust defaults, and durations are validated datetime.timedelta. The bare-address constructor and from_connection_string are unchanged.

One behavior change: a negative timedelta on create_topic / update_topic (message_expiry), the consumer(...) intervals, or AutoCommit.Interval(...) became a near-u64::MAX duration and now raises ValueError.

Local Execution

  • Passed
  • Pre-commit hooks ran

AI Usage

  1. Which tools? Claude
  2. Scope of usage? help generate and review this PR
  3. How did you verify the generated code works correctly? Integration tests against a real server prove credentials are replayed on connect without a manual login_user().
  4. Can you explain every line of the code if asked? Yes, all the changes are checked by the human.

IggyClient accepted only a server address, so auto-login and reconnection
tuning were unreachable from Python. Without credentials to replay, the
SDK's own session recovery never fires and a dropped session surfaces as
Unauthenticated on the next call, leaving the application to hand-roll a
connect/login/probe loop.

TcpConfig mirrors TcpClientConfig field for field and is accepted by the
IggyClient constructor alongside the existing address string. AutoLogin
carries the credentials without exposing them back to Python, and
TcpReconnectionConfig carries the retry policy.

Credentials is re-exported from the SDK prelude because AutoLogin::Enabled
cannot be constructed without naming it.

Closes apache#3742
Round-trip every field through the getters so a default that drifts from
the Rust SDK is caught, and assert that neither the password nor a personal
access token comes back out of repr.

The auto-login tests are the point of the configuration: a privileged call
succeeds without a manual login_user() when credentials are configured, and
fails without them.
The existing examples all reach for a connection string, which leaves the
new config types undiscoverable. This one configures auto-login and
reconnection directly and never calls login_user, so the recovery the
credentials unlock is visible: restart the server while it runs and the
client picks up where it left off.
The README pointed only at the examples directory, so the configuration
surface stayed invisible to anyone reading the package page on PyPI.
A negative timedelta normalizes to negative days plus positive seconds,
so the old conversion summed to a negative i32 and cast it to u64,
turning interval=timedelta(seconds=-1) into u64::MAX seconds: the config
constructed fine and the client then slept forever on reconnect. Days
arithmetic also overflowed i32 beyond ~68 years, and the reverse
conversion stuffed everything into the seconds argument so such values
could not read back. Conversion is now fallible, rejects negative input
with ValueError at construction, computes in i64, and splits days on the
way out. The AutoCommit conversion becomes TryFrom to carry the error.

The boolean constructor defaults were literals in the pyo3 signature, so
a change to a Rust default would silently not propagate. They are now
Option arguments that fall back to TcpClientConfig::default(), the same
way the durations already did.
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.69118% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.57%. Comparing base (01a64b2) to head (9ee182d).

Files with missing lines Patch % Lines
foreign/python/src/config.rs 97.07% 6 Missing ⚠️
foreign/python/src/topic.rs 66.66% 2 Missing ⚠️
foreign/python/src/duration.rs 96.55% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3776      +/-   ##
============================================
- Coverage     76.58%   76.57%   -0.01%     
  Complexity     1046     1046              
============================================
  Files          1347     1349       +2     
  Lines        171018   171242     +224     
  Branches     142372   142448      +76     
============================================
+ Hits         130967   131130     +163     
+ Misses        36233    36199      -34     
- Partials       3818     3913      +95     
Components Coverage Δ
Rust Core 75.78% <ø> (+<0.01%) ⬆️
Java SDK 63.67% <ø> (ø)
C# SDK 71.11% <ø> (-1.17%) ⬇️
Python SDK 89.98% <96.69%> (+1.28%) ⬆️
PHP SDK 82.97% <ø> (ø)
Node SDK 96.38% <ø> (+0.10%) ⬆️
Go SDK 69.18% <ø> (ø)
Files with missing lines Coverage Δ
foreign/python/src/client.rs 99.84% <100.00%> (+0.62%) ⬆️
foreign/python/src/consumer.rs 82.21% <100.00%> (+1.10%) ⬆️
foreign/python/src/lib.rs 100.00% <100.00%> (ø)
foreign/python/src/receive_message.rs 88.37% <ø> (ø)
foreign/python/src/send_message.rs 87.67% <ø> (ø)
foreign/python/src/duration.rs 96.55% <96.55%> (ø)
foreign/python/src/topic.rs 82.27% <66.66%> (-1.06%) ⬇️
foreign/python/src/config.rs 97.07% <97.07%> (ø)

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ethanlin01x
ethanlin01x marked this pull request as ready for review July 29, 2026 18:32
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 29, 2026
@ethanlin01x
ethanlin01x marked this pull request as draft July 29, 2026 18:35
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Jul 29, 2026
conftest auto-marked every module as integration, so tests explicitly
marked unit could not be selected with -m "not integration" even though
they need no server. The auto-mark now skips them.

New cases pin the duration boundaries (negative rejected, zero legal,
beyond the i32 seconds range round-trips) and the README claim that a
connection string and TcpConfig reach the same behavior.
The snippet ended with a top-level await; every other sample in the repo
wraps in asyncio.run, so paste-and-run failed on the only snippet a PyPI
reader sees first.
@ethanlin01x
ethanlin01x force-pushed the feat/python-tcp-config branch from 2b6c6ee to b3190f4 Compare July 29, 2026 18:39
@ethanlin01x
ethanlin01x marked this pull request as ready for review July 29, 2026 19:01
@ethanlin01x

Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 29, 2026

@slbotbm slbotbm left a comment

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.

Looks good. Mostly cosmetic changes. One thing though: in the docs, you are declaring the thrown errors as PyValueError and similar types. These are rust types which the python user will not see. Also, there are references to the rust sdk in public docs. Please remove them. Our modelled users are python users, who would not know anything about rust.

Comment thread examples/python/client-configuration/main.py Outdated
Comment thread examples/python/README.md
Comment thread foreign/python/README.md Outdated
Comment thread foreign/python/README.md Outdated
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jul 29, 2026
Comment thread foreign/python/src/duration.rs Outdated
Comment thread foreign/python/tests/test_client_config.py
Comment thread foreign/python/src/config.rs Outdated
A separate example for the new config is not needed. The getting-started
producer and consumer now build a TcpConfig with auto-login and
reconnection instead of a connection string.
The TLS and nodelay options appear as commented-out fields in the
snippet instead of prose, and the auto_login and from_connection_string
notes are dropped.
Python users see ValueError and RuntimeError rather than the PyO3
exception names, and the wrapped Rust types are an implementation
detail.
Docstrings for methods returning Awaitable[None] said they return Ok(()),
which does not exist for a Python caller. State the raised exception
instead.
IggyDuration::as_micros() truncates the count to u64, so a duration near
timedelta.max wrapped to a wrong value instead of surviving the round
trip, and the OverflowError guard below could never fire. Read the std
Duration directly to keep the u128.
The negative-duration rejection also changed methods that shipped before
this branch, such as create_topic's message_expiry, but only the new
config classes had coverage.
The tls_validate_certificate docstring was neutral for a flag that
accepts any certificate the server presents.
@ethanlin01x

Copy link
Copy Markdown
Contributor Author

Hi @slbotbm
Thanks for the review. Renamed to the Python exception names in 1e5561d, and 42ad9ab clears the remaining Rust the module docstring. The RuntimeError references will need another pass once #3696 lands and Iggy failures become IggyError

@github-actions github-actions Bot added the S-waiting-on-author PR is waiting on author response label Aug 1, 2026
@ethanlin01x

Copy link
Copy Markdown
Contributor Author

/ready

@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Aug 1, 2026
slbotbm
slbotbm previously approved these changes Aug 1, 2026

@hubcio hubcio left a comment

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.

two things that don't fit on diff lines:

  • pre-existing, separate PR: the six sync methods on IggyConsumer (consumer.rs:58-92) call blocking_lock() while holding the GIL; consume_messages holds the same mutex for its whole run, so calling e.g. consumer.name() during consumption deadlocks the interpreter (or panics if called from a sync callback body). untouched by this PR, just surfaced while tracing it.
  • follow-up issue material: the zero-duration hazards below aren't python-specific. TcpClient::create is the one choke point all TcpClientConfig construction sites funnel through (only 2 of 10 go via the builder), and e.g. --tcp-heartbeat-interval none on the CLI already maps to zero today (IggyDuration::from_str treats 0/none/disabled/unlimited the same). a fence there closes every surface at once.

Comment thread foreign/python/src/config.rs Outdated
Comment thread foreign/python/src/config.rs Outdated
Comment thread foreign/python/tests/test_client_config.py Outdated
Comment thread foreign/python/src/client.rs Outdated
Comment thread foreign/python/src/client.rs Outdated
Comment thread foreign/python/src/config.rs Outdated
Comment thread foreign/python/src/config.rs Outdated
Comment thread foreign/python/src/client.rs Outdated
Comment thread examples/python/getting-started/consumer.py Outdated
Comment thread foreign/python/README.md Outdated
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Aug 4, 2026
The topic API landed on master while this branch was in review: message_expiry
and max_topic_size are now IggyExpiry and MaxTopicSize objects rather than a
timedelta and an int, and send_messages returns SendMessagesResponse. Took
those, and the negative-expiry test follows the new type.

Both sides had added their own timedelta conversions, so the two copies master
put in consumer.rs and topic.rs give way to the duration module this branch
introduced, which is where topic.rs now imports both directions from. The
message naming the topic bound is kept at the call site, since the shared
conversion has no way to know what the duration is for.
A zero duration reads as "disabled" nowhere in the client: heartbeat_interval
pings for as long as the client lives, a reconnection interval with unlimited
retries reconnects in a continuous loop, polling_retry_interval spins without a
syscall in the loop body, an AutoCommit interval spins and then floods the
server with offset stores, and init_retry_interval panics inside the runtime
timer without naming the argument that caused it. Each is now rejected where
the sign is already validated, except where zero is meaningful: the cooldown
before reestablishing, a bounded fast-retry interval, and any interval on a
reconnection policy that is switched off.

Building the configuration no longer keeps a second copy of the credentials and
the reconnection policy beside the one the transport reads, no longer rebuilds
the defaults the builder already produced, and no longer routes through a
client builder whose only failure mode cannot happen here. Converting a
timedelta now goes through the conversion pyo3 ships, keeping the message that
does not name Rust types.
The docstrings still described the surface as it behaved before durations were
validated: a negative message_expiry or consumer interval now raises ValueError
at the call rather than becoming a near-maximum duration, and a zero consumer
interval raises it too. A malformed address reaches the user as ValueError
through TcpConfig and as RuntimeError through the string form, which the
constructor documented as one error. tls_ca_file is silently ignored unless
certificate validation is on, and the default reconnection policy retries
forever, so an awaited call never returns while the server is down.

get_stream and get_topic still described their result as an Option, the one
Rust type name left behind when the docstrings were translated for Python
readers.
The repr of a configuration printed five of nine fields, dropping every one a
TLS handshake is debugged with, so a config that accepts any certificate read
exactly like a validating one. Durations printed in a form no constructor
accepts, which cost the repr its one job of being pasteable.

Both examples built their configuration outside the error handling, where an
address without a port reached the user as a traceback rather than as the
message the validation produced.
The maximum-interval test named a u64-microsecond boundary that the interval
never crosses; what it covers is the day conversion in the getter. The
equivalence test claimed both forms of configuration were equivalent while
asserting only that both clients authenticate, which is all the client exposes.
The path was written from the repository root, but the snippet around it is run
from foreign/python, where the certificate is two levels up. The examples readme
already spells it that way.
A max_retries outside the unsigned 32-bit range reached the caller as
OverflowError, raised by the argument conversion before any code here ran, so
it named neither the argument nor the range. OverflowError is not a ValueError,
so a caller guarding construction the way the getting-started examples do never
caught it. The count is now taken wide and narrowed here, where the message can
say which argument it is and what it accepts.
@ethanlin01x

Copy link
Copy Markdown
Contributor Author

@hubcio
Thanks for the thorough review — the pointers into the SDK internals taught me a lot.

On the two that didn't fit on diff lines, the blocking_lock() deadlock and the zero-duration fence at TcpClient::create: want me to open them as follow-up issues?

@ethanlin01x

Copy link
Copy Markdown
Contributor Author

/ready

@ethanlin01x
ethanlin01x requested a review from hubcio August 5, 2026 19:56
@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Aug 5, 2026
@hubcio

hubcio commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@ethanlin01x you can fix them without creation of issue, just mention that it was found in #3776.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python SDK: IggyClient exposes no client configuration — no reconnection or auto-login, so session recovery is unreachable (Go/C# already expose it)

4 participants