feat(python): expose TCP client configuration - #3776
Open
ethanlin01x wants to merge 7 commits into
Open
Conversation
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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3776 +/- ##
============================================
- Coverage 75.10% 75.08% -0.03%
Complexity 969 969
============================================
Files 1307 1309 +2
Lines 152751 152950 +199
Branches 128185 128259 +74
============================================
+ Hits 114726 114835 +109
- Misses 34494 34497 +3
- Partials 3531 3618 +87
🚀 New features to boost your workflow:
|
ethanlin01x
marked this pull request as ready for review
July 29, 2026 18:32
ethanlin01x
marked this pull request as draft
July 29, 2026 18:35
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
force-pushed
the
feat/python-tcp-config
branch
from
July 29, 2026 18:39
2b6c6ee to
b3190f4
Compare
ethanlin01x
marked this pull request as ready for review
July 29, 2026 19:01
Contributor
Author
|
/ready |
slbotbm
requested changes
Jul 29, 2026
slbotbm
left a comment
slbotbm
left a comment
Contributor
There was a problem hiding this comment.
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.
Contributor
There was a problem hiding this comment.
There's no need for an extra example for the new config. Let's fold this into the getting-started/ example
Contributor
There was a problem hiding this comment.
No need for this extra change if the above example gets folded in getting-started.
Comment on lines
+94
to
+97
| `TcpConfig` also carries `tls_enabled`, `tls_domain`, `tls_ca_file`, | ||
| `tls_validate_certificate` and `nodelay`. Every field is keyword-only and defaults to the | ||
| same value the Rust SDK uses. `IggyClient.from_connection_string(...)` remains available | ||
| for the same settings in string form. |
Contributor
There was a problem hiding this comment.
You could show these as commented out options in the above config instead of mentioning them here. No need to mention anything about from_connection_string here.
Comment on lines
+63
to
+65
| `IggyClient` takes either a server address or a `TcpConfig`. Configuring `auto_login` | ||
| lets the SDK replay the credentials whenever it reconnects, so a session dropped by a | ||
| server restart is recovered instead of surfacing as `Unauthenticated`: |
Contributor
There was a problem hiding this comment.
No need to mention the auto_login option specially. That is for the API comment docs.
github-actions
Bot
added
S-waiting-on-author
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
2 participants
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.
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, which makes the SDK's session recovery (#2880) unreachable: a session dropped by a server restart surfaces as
Unauthenticatedon the next call and applications have to hand-roll connect/login/probe retry loops.What changed?
IggyClient(...)only tookhost:port, withAutoLogin::Disabledhardcoded and the reconnection policy untunable. It now also accepts aTcpConfigmirroring the RustTcpClientConfig(auto_login,reconnection,heartbeat_interval, the TLS options,nodelay), keyword-only, with every unset field falling back to the Rust default instead of a value duplicated in the binding. Durations aredatetime.timedeltavalidated at construction, which also fixes the pre-existing conversion that cast a negative timedelta into a huge unsigned duration. Type names follow the maintainer's guidance in the issue (TcpConfig,TcpReconnectionConfig); scope is TCP only, and the bare-address constructor andfrom_connection_stringare unchanged.Local Execution
AI Usage
login_user().