Skip to content

feat(client): add connect_timeout to bound the connect phase - #1324

Draft
fatih-acar wants to merge 1 commit into
stablefrom
fac/dns-round-robin-5x8wr
Draft

feat(client): add connect_timeout to bound the connect phase#1324
fatih-acar wants to merge 1 commit into
stablefrom
fac/dns-round-robin-5x8wr

Conversation

@fatih-acar

@fatih-acar fatih-acar commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

An Infrahub endpoint that does not answer the TCP or TLS handshake used to hold every SDK request for the full request timeout (60 seconds by default) before failing. This PR bounds the connect phase separately, so a client behind a DNS round robin or a failing-over load balancer moves on to its next attempt after a few seconds instead of waiting out the whole request budget.

Key Changes

  • A new connect_timeout setting (default 10 seconds, INFRAHUB_CONNECT_TIMEOUT) bounds the TCP/TLS handshake on every request path: regular requests, multipart uploads and streaming downloads, on both the async and sync clients. It never exceeds the per-request timeout.
  • The timeout setting's description now states what it covers (read, write and pool phases) instead of calling itself a "connection timeout". No behaviour change.

The transient retry handler on stable already treats a connect timeout as a lost connection and retries it when retry_on_failure is enabled, so this PR only shortens how long each failed attempt takes. An earlier revision of this PR also carried that retry mapping; it was dropped during the rebase onto stable.

Documentation Updates

  • docs/docs/python-sdk/reference/config.mdx regenerated with the new setting.
  • One changelog fragment in changelog/.

Test Plan

  • uv run pytest tests/unit/sdk/test_connect_timeout.py covers the config default and validation, the exact timeout httpx receives on every path for both clients, the cap by the request timeout, and that a connect timeout still surfaces as ServerNotReachableError and is retried through execute_graphql.
  • uv run invoke format lint-code and uv run invoke docs-validate pass locally.
  • The full tests/unit suite passes except three pre-existing infrahubctl table-rendering failures that fail identically on stable.

The SDK passed a single integer timeout to httpx, which applied it to
the connect phase as well, so an unreachable address consumed the whole
request budget (60s by default) before failing. A new connect_timeout
setting (default 10s, INFRAHUB_CONNECT_TIMEOUT) now bounds the TCP/TLS
handshake on every request path, capped by the per-request timeout.

Since the transient retry handler already treats a connect timeout as a
lost connection, a client behind a DNS round robin or a failing-over
load balancer now moves on to its next attempt after connect_timeout
seconds instead of waiting out the full request timeout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@fatih-acar fatih-acar added the type/feature New feature or request label Sep 4, 2026
@github-actions github-actions Bot added the type/documentation Improvements or additions to documentation label Sep 4, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 4, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: a40ccf7
Status: ✅  Deploy successful!
Preview URL: https://87dd73a0.infrahub-sdk-python.pages.dev
Branch Preview URL: https://fac-dns-round-robin-5x8wr.infrahub-sdk-python.pages.dev

View logs

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##           stable    #1324      +/-   ##
==========================================
- Coverage   84.88%   84.86%   -0.02%     
==========================================
  Files         148      148              
  Lines       13309    13291      -18     
  Branches     1965     1955      -10     
==========================================
- Hits        11297    11280      -17     
  Misses       1442     1442              
+ Partials      570      569       -1     
Flag Coverage Δ
integration-tests 38.76% <50.00%> (-0.09%) ⬇️
python-3.10 57.72% <50.00%> (+0.07%) ⬆️
python-3.11 57.72% <50.00%> (+0.05%) ⬆️
python-3.12 57.72% <50.00%> (+0.05%) ⬆️
python-3.13 57.72% <50.00%> (+0.07%) ⬆️
python-3.14 57.72% <50.00%> (+0.05%) ⬆️
python-filler-3.12 23.78% <50.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/client.py 84.06% <100.00%> (+0.02%) ⬆️
infrahub_sdk/config.py 91.61% <100.00%> (+0.05%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 6 files

Re-trigger cubic

@fatih-acar fatih-acar changed the title feat(client): add connect_timeout and retry on httpx.ConnectTimeout feat(client): add connect_timeout to bound the connect phase Sep 8, 2026
@fatih-acar
fatih-acar force-pushed the fac/dns-round-robin-5x8wr branch from 2a84404 to a40ccf7 Compare September 8, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant