Skip to content

feat(rust): name how long an idle pooled connection is kept - #741

Draft
wkirschenmann wants to merge 3 commits into
wk/feat/rust-schema-doc-linksfrom
wk/feat/rust-pool-idle-timeout
Draft

wkirschenmann wants to merge 3 commits into
wk/feat/rust-schema-doc-linksfrom
wk/feat/rust-pool-idle-timeout

Conversation

@wkirschenmann

@wkirschenmann wkirschenmann commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Sits on #738, which has to merge first.

Motivation

A schema description reaches a generated options class verbatim, so what the vocabulary spells is
what a consumer writes. One description names a Rust field no deployment can set, and a pooled
deployment has no option at all for how long an idle connection is kept.

Description

PoolIdleTimeout joins the vocabulary at the root, under no prefix: it bounds a connection's idle
life in a pool, which is neither a TCP nor an HTTP/2 setting. Nothing in this crate applies it, on
the model the retry field already sets - a channel is one connection and has no pool, so connect
has nothing to hand it to, and whoever drives the pool reads it.

Schema descriptions then spell the flat option name instead of the Rust field: allow_unsafe_connection
becomes AllowUnsafeConnection. Where the vocabulary cannot place a link, it keeps exactly the
rendering it has today, its last segment in backticks - the change never invents a name.

The prefix is the trap: a field of an embedded unit is spelled with its embedding's prefix, so
keep_alive_interval of Http2Config is Http2KeepAliveInterval. The units, their prefixes and
their option fields are declared in config.rs next to the embed_prefixed! calls that choose
those prefixes, and config_utils learns no vocabulary. They are listed rather than inferred
because a rustdoc path says nothing about what it landed on: ProxyConfig::username and
ProxyConfig::explicit are spelled alike, so inference would render ProxyExplicit, the very
defect being fixed.

Testing

Adds 5. One reads PoolIdleTimeout from the environment, the only thing that proves an option
exists when no code consumes the value. Four cover the rewriting, among them a drift check walking
the hand-written table against the generated schema and requiring every listed field to render a
name the schema declares; mutated with the proxy prefix written Prox, it fails as it should.

cargo test -p armonik-transport --all-features, all green.

Impact

Descriptions change wording and the vocabulary gains one name; no existing option moves.

Additional Information

Two commits in one PR because they share a subject: both change what the option vocabulary tells the
consumer generating a class from it, which was wrong in the two ways it can be - a name missing from
it, and a description naming something no deployment can spell. Neither depends on the other.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • I have thoroughly tested my modifications and added tests when necessary.
  • Tests pass locally and in the CI. (locally yes; CI has not run this branch)
  • I have assessed the performance impact of my modifications. (nothing measured)

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
1478 1247 84% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: 6f42686 by action🐍

@wkirschenmann
wkirschenmann force-pushed the wk/feat/rust-pool-idle-timeout branch from a5ed6df to 8cb34ff Compare August 9, 2026 08:37
`PoolIdleTimeout` joins the flat vocabulary at the root, under no prefix: it bounds a connection's
idle life in a pool, which is neither a TCP nor an HTTP/2 setting.

Nothing here applies it, the same way `MaxAttempts` and its neighbours are not applied here: a
channel is one connection and has no pool, so `connect` has nothing to hand it to. Whoever drives
a pool reads it - `hyper-util`'s legacy client takes it as `pool_idle_timeout`.

Verified from packages/rust:

    cargo test -p armonik-transport --all-features
      101 unit, 15 proxy, 7 schema, 6 timeout, 0 doc; all passed
    cargo clippy -p armonik-transport --all-targets --all-features -- -D warnings
      clean
    cargo fmt --check
      clean
    cargo build --workspace --all-features
      ok
A description reaches a generated options class verbatim, so `allow_unsafe_connection` in one names
an option no deployment can set: every source spells it `AllowUnsafeConnection`. The rewriting of
intra-doc links now renders the flat name where it can place the link, and keeps its old rendering
where it cannot.

The prefix is the trap: a field of an embedded unit is spelled with its embedding's prefix, so a
`keep_alive_interval` of `Http2Config` is `Http2KeepAliveInterval` and never the bare field name.
`config_utils` keeps the mechanism and learns no vocabulary: the units, their prefixes and their
option fields are declared in `config.rs`, next to the `embed_prefixed!` calls that choose those
prefixes, and reach the transform as a parameter.

The fields are listed rather than inferred. A rustdoc path says nothing about what it landed on -
`ProxyConfig::username` and `ProxyConfig::explicit` are spelled alike - and neither does casing, so
inferring would render `ProxyExplicit`, a name no source spells and the very defect this fixes.
Unlisted, a link keeps its last segment in backticks as before. A test ties the table back to the
generated schema: every listed field has to render a name the schema really declares.

Verified from packages/rust:

    cargo test -p armonik-transport --all-features
      104 unit, 15 proxy, 8 schema, 6 timeout, 0 doc; all passed
    cargo clippy -p armonik-transport --all-targets --all-features -- -D warnings
      clean
    cargo fmt --check
      clean
    cargo build --workspace --all-features
      ok

The drift test was checked against a mutation: with the proxy prefix written `Prox`, it fails with
"`ProxyConfig::username` renders `ProxUsername`, which no option spells".
@wkirschenmann
wkirschenmann force-pushed the wk/feat/rust-pool-idle-timeout branch from 8cb34ff to 9a5f2f4 Compare August 9, 2026 10:40
`CaCertPath` names the path an authority is read from, while
`TlsConfig::ca_cert` holds the certificate the reader loaded from it. The
table says every field it lists names one option, so listing this one made a
link to it render `CaCert`, a name no deployment can set - the very thing the
rewriting exists to keep out of the schema.

Caught by every_field_a_link_resolves_through_names_an_option_the_schema_declares,
which is the check the table was given for exactly this drift. No doc comment
links to the field, so nothing renders differently.
@sonarqubecloud

sonarqubecloud Bot commented Aug 9, 2026

Copy link
Copy Markdown

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.

1 participant