Skip to content

feat(python): add WebSocketConfig transport configuration - #4000

Open
saie-ch wants to merge 1 commit into
apache:masterfrom
saie-ch:python-websocket-config
Open

feat(python): add WebSocketConfig transport configuration#4000
saie-ch wants to merge 1 commit into
apache:masterfrom
saie-ch:python-websocket-config

Conversation

@saie-ch

@saie-ch saie-ch commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Relates to #2835.

Rationale

Adds WebSocket transport support to the Python SDK, following the same Config pattern already used for TCP.

What changed?

Adds WebSocketConfig, WebSocketReconnectionConfig, and WebSocketFramingConfig, plus IggyClient.websocket(config). Mirrors TcpConfig's fields and validation.

Local Execution

  • Passed
  • Pre-commit hooks ran

AI Usage
Claude Sonnet 5 (Claude Code)

@github-actions

Copy link
Copy Markdown

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

Slash commands (own line, regular comment) move it around the queue:

  • /ready - back to S-waiting-on-review after addressing feedback
  • /author - flip to S-waiting-on-author while you finish changes
  • /request-review @user-or-team - request a reviewer

See CONTRIBUTING.md for details.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 31, 2026
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.91120% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.92%. Comparing base (5916e6f) to head (e4e4716).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
foreign/python/src/config.rs 96.72% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             master    #4000    +/-   ##
==========================================
  Coverage     84.91%   84.92%            
  Complexity     1405     1405            
==========================================
  Files          1224     1224            
  Lines        179301   179560   +259     
  Branches     145615   145614     -1     
==========================================
+ Hits         152250   152485   +235     
- Misses        23024    23029     +5     
- Partials       4027     4046    +19     
Components Coverage Δ
Rust Core 85.77% <ø> (-0.03%) ⬇️
Java SDK 67.35% <ø> (ø)
C# SDK 75.44% <ø> (+0.04%) ⬆️
Python SDK 90.76% <96.91%> (+0.69%) ⬆️
PHP SDK 85.65% <ø> (ø)
Node SDK 96.24% <ø> (+0.11%) ⬆️
Go SDK 69.29% <ø> (+0.03%) ⬆️
Files with missing lines Coverage Δ
foreign/python/src/client.rs 99.86% <100.00%> (+<0.01%) ⬆️
foreign/python/src/lib.rs 100.00% <100.00%> (ø)
foreign/python/src/config.rs 96.86% <96.72%> (-0.17%) ⬇️

... and 43 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.

@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.

Other than the comments below, the API exposed in this PR does not match the established pattern, which is IggyClient(server_address: str | TcpConfig | ...). Let's conform to that pattern instead of exposing .websocket method.

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 need for examples like these. You can add comments to the existing examples on how to configure different transports instead.

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.

Not needed.

inner.max_write_buffer_size =
Some(usize_param(max_write_buffer_size, "max_write_buffer_size")?);
}
if let Some(max_message_size) = max_message_size {

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.

Passing None for max_message_size or max_frame_size cannot disable the limit as documented. An omitted argument and an explicit Python None both arrive as Option<i64>::None, the constructor starts from the Rust defaults of 64 MiB and 16 MiB, and these assignments run only for Some. The Rust to_tungstenite_config() conversion also leaves tungstenite's default in place when its field is None. A caller that requests no limit will therefore still have large messages or frames rejected. Let's distinguish an omitted argument from an explicit None and make the Rust conversion call the tungstenite setters with None.

@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 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author PR is waiting on author response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants