hubcio commented on code in PR #3992:
URL: https://github.com/apache/iggy/pull/3992#discussion_r3941913388
##########
foreign/python/README.md:
##########
@@ -168,6 +168,11 @@ async def main():
asyncio.run(main())
```
+`IggyClient(...)` also accepts an `HttpConfig` for the HTTP transport. HTTP is
a
Review Comment:
warning: consumer groups don't work over HTTP - `consumer_group(...)`
defaults to auto-join, and `join_consumer_group` returns `Feature is
unavailable`, so `init()` throws. list that here.
##########
examples/python/getting-started/producer.py:
##########
@@ -113,6 +113,13 @@ def build_config(args: ArgNamespace) -> TcpConfig:
tls_enabled=args.tls,
tls_ca_file=args.tls_ca_file or None,
)
+ # IggyClient(...) also accepts an HttpConfig for the HTTP transport. HTTP
Review Comment:
warning: this sits after `return TcpConfig(...)`, so uncommenting it doesn't
switch transport - the example silently keeps running over TCP. move it above
the return and widen the annotation to `TcpConfig | HttpConfig`.
same block at `consumer.py:117`.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
Review Comment:
warning: a group poll over HTTP returns `[]` forever with no error - the
server can't register an HTTP client, so it fences and answers 200 with an
empty list. say HTTP is single-consumer only.
##########
foreign/python/README.md:
##########
@@ -168,6 +168,11 @@ async def main():
asyncio.run(main())
```
+`IggyClient(...)` also accepts an `HttpConfig` for the HTTP transport. HTTP is
a
+stateless per-request transport, so there is no reconnection policy to
configure.
+There is also no `AutoLogin`: call `login_user(...)` explicitly after
connecting.
+See `examples/python/getting-started/producer.py` for a config swap example.
Review Comment:
nit: that block is commented-out code sitting after a `return`, so it isn't
a usable example. inline the four lines here instead.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
Review Comment:
warning: `validate_api_url` rejects any path, query, fragment or
credentials, so `https://gw.example.com/iggy` fails with a reasonless
`ValueError`. say the accepted shape is `scheme://host[:port]` only, and add a
path case to `test_invalid_api_url_is_rejected`.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
+ /// jwt: JWT token for A2A (Agent-to-Agent) authentication. Defaults
to `None`.
+ /// heartbeat_interval: Interval of heartbeats sent by the client.
Defaults to 5 seconds.
Review Comment:
nit: copied from `TcpConfig`, but over HTTP the ping is a bare `GET /ping`
against a handler that registers nothing - it keeps no session alive. say it's
a liveness probe only.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
+ /// jwt: JWT token for A2A (Agent-to-Agent) authentication. Defaults
to `None`.
+ /// heartbeat_interval: Interval of heartbeats sent by the client.
Defaults to 5 seconds.
+ ///
+ /// Raises:
+ /// ValueError: If `api_url` is not a valid URL, if `retries` is
outside the
+ /// range of an unsigned 32-bit integer, if a duration is
negative, or
+ /// if `heartbeat_interval` is zero.
+ #[new]
+ #[pyo3(signature = (*, api_url=None, retries=None, jwt=None,
heartbeat_interval=None))]
+ fn new(
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] api_url:
Option<String>,
+ #[gen_stub(override_type(type_repr = "builtins.int | None"))] retries:
Option<i64>,
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] jwt:
Option<String>,
+ #[gen_stub(override_type(type_repr = "datetime.timedelta | None",
imports=("datetime")))]
+ heartbeat_interval: Option<Py<PyDelta>>,
+ ) -> PyResult<Self> {
+ // The builder starts from `HttpClientConfig::default()`, and its
`build()`
+ // trims and validates the API URL whether or not one was set here.
+ let mut builder = HttpClientConfigBuilder::new();
+ if let Some(api_url) = api_url {
+ builder = builder.with_api_url(api_url);
+ }
+ if let Some(retries) = retries {
+ let retries = u32::try_from(retries).map_err(|_| {
+ PyValueError::new_err(format!("'retries' must be between 0 and
{}", u32::MAX))
+ })?;
+ builder = builder.with_retries(retries);
+ }
+ if let Some(jwt) = jwt {
Review Comment:
warning: `jwt=""` is accepted, so `has_jwt` says True while
`is_authenticated()` is false and every call fails `Unauthenticated`.
whitespace is worse - it passes the local check and 401s server-side. reject
empty and whitespace here.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
+ /// jwt: JWT token for A2A (Agent-to-Agent) authentication. Defaults
to `None`.
+ /// heartbeat_interval: Interval of heartbeats sent by the client.
Defaults to 5 seconds.
+ ///
+ /// Raises:
+ /// ValueError: If `api_url` is not a valid URL, if `retries` is
outside the
Review Comment:
nit: `retries` is `Option<i64>`, so `retries=2**64` raises `OverflowError`
from pyo3 before this check runs. name `OverflowError` here, or take a
`&Bound<'_, PyAny>` and range-check it.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
Review Comment:
warning: defaults to 3 and reqwest-retry replays POST, so a 504 on produce
appends the batch twice - the server mints a fresh request id per request, so
dedup misses it. document that HTTP sends are at-least-once and that
`retries=0` restores TCP semantics.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -0,0 +1,190 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests for the HTTP client configuration surface.
+
+`HttpConfig` mirrors the Rust SDK's `HttpClientConfig` the same way
+`TcpConfig`/`QuicConfig` do, so most of these assert that a value set from
Review Comment:
nit: no `QuicConfig` is bound - `lib.rs` registers only `AutoLogin`,
`TcpConfig`, `TcpReconnectionConfig` and `HttpConfig`. drop it here; line 24's
"unlike TCP/QUIC" is fine, that's true of the transports.
##########
core/sdk/src/prelude.rs:
##########
@@ -41,6 +41,7 @@ pub use crate::clients::producer_builder::IggyProducerBuilder;
pub use crate::clients::producer_config::{BackgroundConfig, DirectConfig};
pub use crate::clients::producer_sharding::{BalancedSharding, OrderedSharding,
Sharding};
pub use crate::consumer_ext::IggyConsumerMessageExt;
+pub use crate::http::http_client::HttpClient;
Review Comment:
nit: the prelude now carries clients for http, tcp and websocket but not
`QuicClient`, though `QuicClientConfig` is already here. add it, or drop this
line and import `HttpClient` from its module in the binding.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -0,0 +1,190 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests for the HTTP client configuration surface.
+
+`HttpConfig` mirrors the Rust SDK's `HttpClientConfig` the same way
+`TcpConfig`/`QuicConfig` do, so most of these assert that a value set from
+Python survives to the getters and that unset fields fall back to the Rust
+defaults. HTTP is stateless per-request, so unlike TCP/QUIC there is no
+`AutoLogin` or reconnection policy to configure.
+"""
+
+import ast
+from datetime import timedelta
+
+import pytest
+
+from apache_iggy import Consumer, HttpConfig, IggyClient, PollingStrategy
+from apache_iggy import SendMessage as Message
+
+from .utils import get_http_server_config, wait_for_ping
+
+
[email protected]
+class TestHttpConfig:
+ """Test the transport configuration."""
+
+ def test_defaults_match_the_rust_sdk(self):
+ """Test that an unconfigured transport matches the Rust SDK
defaults."""
+ config = HttpConfig()
+
+ assert config.api_url == "http://127.0.0.1:3000";
+ assert config.retries == 3
+ assert config.has_jwt is False
+ assert config.heartbeat_interval == timedelta(seconds=5)
+
+ def test_every_field_round_trips(self):
+ """Test that each configured field is readable back unchanged."""
+ config = HttpConfig(
+ api_url="http://127.0.0.1:3001";,
+ retries=5,
+ jwt="a-token",
+ heartbeat_interval=timedelta(seconds=15),
+ )
+
+ assert config.api_url == "http://127.0.0.1:3001";
+ assert config.retries == 5
+ assert config.has_jwt is True
Review Comment:
nit: `has_jwt` only proves the flag, nothing checks the token reaches
`access_token`. `/users/login` is public, so stdlib `urllib` can mint one out
of band and assert an authenticated call works.
##########
.github/actions/python-maturin/pre-merge/action.yml:
##########
@@ -171,6 +171,7 @@ runs:
# overwrite the coverage-instrumented .so with a non-instrumented one
IGGY_SERVER_HOST=127.0.0.1 \
IGGY_SERVER_TCP_PORT=8090 \
+ IGGY_SERVER_HTTP_PORT=3000 \
Review Comment:
nit: host and both ports duplicate `server-start`'s defaults, so changing
one desyncs silently. if you wire the outputs, split them first -
`http_address` is `host:port`, and `int()` on that raises in `utils.py`.
##########
foreign/python/README.md:
##########
@@ -168,6 +168,11 @@ async def main():
asyncio.run(main())
```
+`IggyClient(...)` also accepts an `HttpConfig` for the HTTP transport. HTTP is
a
+stateless per-request transport, so there is no reconnection policy to
configure.
+There is also no `AutoLogin`: call `login_user(...)` explicitly after
connecting.
Review Comment:
nit: over HTTP `connect()` is a no-op that never dials, so "after
connecting" misleads. the SDK's own wording is "call `login_user()` instead of
`connect()`".
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
+ /// jwt: JWT token for A2A (Agent-to-Agent) authentication. Defaults
to `None`.
+ /// heartbeat_interval: Interval of heartbeats sent by the client.
Defaults to 5 seconds.
+ ///
+ /// Raises:
+ /// ValueError: If `api_url` is not a valid URL, if `retries` is
outside the
+ /// range of an unsigned 32-bit integer, if a duration is
negative, or
+ /// if `heartbeat_interval` is zero.
+ #[new]
+ #[pyo3(signature = (*, api_url=None, retries=None, jwt=None,
heartbeat_interval=None))]
+ fn new(
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] api_url:
Option<String>,
+ #[gen_stub(override_type(type_repr = "builtins.int | None"))] retries:
Option<i64>,
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] jwt:
Option<String>,
+ #[gen_stub(override_type(type_repr = "datetime.timedelta | None",
imports=("datetime")))]
+ heartbeat_interval: Option<Py<PyDelta>>,
+ ) -> PyResult<Self> {
+ // The builder starts from `HttpClientConfig::default()`, and its
`build()`
+ // trims and validates the API URL whether or not one was set here.
+ let mut builder = HttpClientConfigBuilder::new();
+ if let Some(api_url) = api_url {
+ builder = builder.with_api_url(api_url);
+ }
+ if let Some(retries) = retries {
+ let retries = u32::try_from(retries).map_err(|_| {
Review Comment:
simplification: same conversion and error template as
`TcpReconnectionConfig::new` above. a private `u32_arg(value, parameter)` in
this file covers both, and gives the `OverflowError` fix one place to live.
##########
foreign/python/src/client.rs:
##########
@@ -86,45 +86,73 @@ fn resolve_topic_params(
#[gen_stub_pymethods]
#[pymethods]
impl IggyClient {
- /// Constructs a new IggyClient from a TCP server address or a `TcpConfig`.
- /// This initializes a new runtime for asynchronous operations.
+ /// Constructs a new IggyClient from a TCP server address, a `TcpConfig`,
or an
+ /// `HttpConfig`. This initializes a new runtime for asynchronous
operations.
/// Future versions might utilize asyncio for more Pythonic async.
///
/// Args:
- /// conn: Either a `host:port` address, or a `TcpConfig` carrying the
full
- /// transport configuration. Defaults to `127.0.0.1:8090` with
auto-login
- /// disabled. A malformed address is reported differently by the
two
- /// forms: the string form raises `RuntimeError` here, while
`TcpConfig`
- /// raises `ValueError` when it is constructed, before it ever
reaches
- /// this call. Neither exception is a subclass of the other.
+ /// conn: A `host:port` address, a `TcpConfig`, or an `HttpConfig`.
Defaults
+ /// to `127.0.0.1:8090` over TCP with auto-login disabled. A
malformed
+ /// address is reported differently depending on the form: the
string
+ /// form raises `RuntimeError` here, while `TcpConfig`/`HttpConfig`
+ /// raise `ValueError` when they are constructed, before either
ever
+ /// reaches this call. Neither exception is a subclass of the
other.
///
/// Raises:
/// RuntimeError: If the address passed as a string is not a valid
- /// `host:port` pair.
+ /// `host:port` pair, or if an `HttpConfig` client cannot be
+ /// constructed. `api_url` is already validated when `HttpConfig`
is
+ /// built, so the latter does not currently fail; the exception is
+ /// documented for interface consistency with the other transports.
#[new]
#[pyo3(signature = (conn=None))]
fn new(
- #[gen_stub(override_type(type_repr = "TcpConfig | builtins.str |
None"))] conn: Option<
- PyClientConfig,
- >,
+ #[gen_stub(override_type(type_repr = "TcpConfig | HttpConfig |
builtins.str | None"))]
+ conn: Option<PyClientConfig>,
) -> PyResult<Self> {
- let config = match conn {
- Some(PyClientConfig::Config(config)) => config.client_config(),
- Some(PyClientConfig::ServerAddress(server_address)) => Arc::new(
- TcpClientConfigBuilder::new()
- .with_server_address(server_address)
- .build()
- .map_err(|e| {
+ match conn {
Review Comment:
simplification: four arms each end in the same `Ok(Self { inner:
Arc::new(RustIggyClient::new(...)) })`, and the `PyErr::new` closure is written
five times. let the match yield a `ClientWrapper` and construct once after it.
##########
foreign/python/src/client.rs:
##########
@@ -86,45 +86,73 @@ fn resolve_topic_params(
#[gen_stub_pymethods]
#[pymethods]
impl IggyClient {
- /// Constructs a new IggyClient from a TCP server address or a `TcpConfig`.
- /// This initializes a new runtime for asynchronous operations.
+ /// Constructs a new IggyClient from a TCP server address, a `TcpConfig`,
or an
+ /// `HttpConfig`. This initializes a new runtime for asynchronous
operations.
/// Future versions might utilize asyncio for more Pythonic async.
///
/// Args:
- /// conn: Either a `host:port` address, or a `TcpConfig` carrying the
full
- /// transport configuration. Defaults to `127.0.0.1:8090` with
auto-login
- /// disabled. A malformed address is reported differently by the
two
- /// forms: the string form raises `RuntimeError` here, while
`TcpConfig`
- /// raises `ValueError` when it is constructed, before it ever
reaches
- /// this call. Neither exception is a subclass of the other.
+ /// conn: A `host:port` address, a `TcpConfig`, or an `HttpConfig`.
Defaults
+ /// to `127.0.0.1:8090` over TCP with auto-login disabled. A
malformed
+ /// address is reported differently depending on the form: the
string
+ /// form raises `RuntimeError` here, while `TcpConfig`/`HttpConfig`
+ /// raise `ValueError` when they are constructed, before either
ever
+ /// reaches this call. Neither exception is a subclass of the
other.
///
/// Raises:
Review Comment:
simplification: the `HttpConfig` half can't fire - `build()` already
validated `api_url` and there's no setter, so `create` re-checks the same
string. keep the string-address clause, drop the justification.
##########
foreign/python/tests/utils.py:
##########
@@ -58,6 +62,26 @@ def get_server_config() -> tuple[str, int]:
return host, port
+def get_server_config() -> tuple[str, int]:
+ """
+ Get TCP server configuration from environment variables or defaults.
+
+ Returns:
+ tuple: (host, port) for the Iggy server
+ """
+ return get_transport_config("IGGY_SERVER_TCP_PORT", 8090)
+
+
+def get_http_server_config() -> tuple[str, int]:
Review Comment:
simplification: a one-line delegation with a six-line docstring and two call
sites. give `get_server_config` default args instead, or call
`get_transport_config` directly at both sites.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -0,0 +1,190 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests for the HTTP client configuration surface.
+
+`HttpConfig` mirrors the Rust SDK's `HttpClientConfig` the same way
+`TcpConfig`/`QuicConfig` do, so most of these assert that a value set from
+Python survives to the getters and that unset fields fall back to the Rust
+defaults. HTTP is stateless per-request, so unlike TCP/QUIC there is no
+`AutoLogin` or reconnection policy to configure.
+"""
+
+import ast
+from datetime import timedelta
+
+import pytest
+
+from apache_iggy import Consumer, HttpConfig, IggyClient, PollingStrategy
+from apache_iggy import SendMessage as Message
+
+from .utils import get_http_server_config, wait_for_ping
+
+
[email protected]
+class TestHttpConfig:
+ """Test the transport configuration."""
+
+ def test_defaults_match_the_rust_sdk(self):
+ """Test that an unconfigured transport matches the Rust SDK
defaults."""
+ config = HttpConfig()
+
+ assert config.api_url == "http://127.0.0.1:3000";
+ assert config.retries == 3
+ assert config.has_jwt is False
+ assert config.heartbeat_interval == timedelta(seconds=5)
+
+ def test_every_field_round_trips(self):
+ """Test that each configured field is readable back unchanged."""
+ config = HttpConfig(
+ api_url="http://127.0.0.1:3001";,
+ retries=5,
+ jwt="a-token",
+ heartbeat_interval=timedelta(seconds=15),
+ )
+
+ assert config.api_url == "http://127.0.0.1:3001";
+ assert config.retries == 5
+ assert config.has_jwt is True
+ assert config.heartbeat_interval == timedelta(seconds=15)
+
+ def test_arguments_are_keyword_only(self):
+ """Test that the API URL cannot be passed positionally."""
+ with pytest.raises(TypeError):
+ # pyrefly: ignore # bad-argument-count
+ HttpConfig("http://127.0.0.1:3000";)
+
+ def test_repr_hides_the_jwt(self):
+ """Test that the JWT does not leak through repr but still parses as
Python."""
+ config = HttpConfig(jwt="a-secret-token")
+
+ printed = repr(config)
+
+ assert "a-secret-token" not in printed
+ ast.parse(printed)
+
+ def test_repr_shows_every_field_as_python(self):
+ """Test that repr covers the configured fields and parses as Python."""
+ config = HttpConfig(api_url="http://127.0.0.1:3001";, retries=5)
+
+ printed = repr(config)
+
+ assert 'api_url="http://127.0.0.1:3001";' in printed
+ assert "retries=5" in printed
+ ast.parse(printed)
+
+ @pytest.mark.parametrize(
+ "invalid_url",
+ ["", "not-a-url", "http://127.0.0.1:0";],
+ )
+ def test_invalid_api_url_is_rejected(self, invalid_url: str):
+ """Test that a malformed API URL fails at construction, not at
connect."""
+ with pytest.raises(ValueError):
+ HttpConfig(api_url=invalid_url)
+
+ @pytest.mark.parametrize("out_of_range", [-1, 2**32])
+ def test_out_of_range_retries_is_rejected(self, out_of_range: int):
+ """Test that a retry count outside the wire range names the argument.
+
+ The conversion pyo3 does on its own raises OverflowError, which is not
a
+ ValueError and so escapes the handler a caller wraps construction in.
+ """
+ with pytest.raises(ValueError, match="retries"):
+ HttpConfig(retries=out_of_range)
+
+ def test_negative_heartbeat_interval_is_rejected(self):
+ """Test that a negative heartbeat interval fails at construction."""
+ with pytest.raises(ValueError, match="negative"):
+ HttpConfig(heartbeat_interval=timedelta(seconds=-3))
+
+ def test_zero_heartbeat_interval_is_rejected(self):
+ """Test that a zero heartbeat interval fails at construction.
+
+ Nothing downstream reads zero as "disabled"; it heartbeats in a
+ continuous loop for as long as the client lives.
+ """
+ with pytest.raises(ValueError, match="zero"):
+ HttpConfig(heartbeat_interval=timedelta(0))
+
+
[email protected]
+class TestHttpClientConstruction:
+ """Test that `IggyClient(...)` accepts an `HttpConfig`."""
+
+ def test_accepts_a_config(self):
+ """Test that a client can be built from a config object."""
+ assert IggyClient(HttpConfig(api_url="http://127.0.0.1:3000";)) is not
None
+
+ def test_accepts_the_default_config(self):
Review Comment:
simplification: this is the same test as the one above - `HttpConfig()`
defaults `api_url` to that exact literal, and both hit the same union arm. keep
one, once the transport assertion is in.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -0,0 +1,190 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests for the HTTP client configuration surface.
+
+`HttpConfig` mirrors the Rust SDK's `HttpClientConfig` the same way
+`TcpConfig`/`QuicConfig` do, so most of these assert that a value set from
+Python survives to the getters and that unset fields fall back to the Rust
+defaults. HTTP is stateless per-request, so unlike TCP/QUIC there is no
+`AutoLogin` or reconnection policy to configure.
+"""
+
+import ast
+from datetime import timedelta
+
+import pytest
+
+from apache_iggy import Consumer, HttpConfig, IggyClient, PollingStrategy
+from apache_iggy import SendMessage as Message
+
+from .utils import get_http_server_config, wait_for_ping
+
+
[email protected]
+class TestHttpConfig:
+ """Test the transport configuration."""
+
+ def test_defaults_match_the_rust_sdk(self):
+ """Test that an unconfigured transport matches the Rust SDK
defaults."""
+ config = HttpConfig()
+
+ assert config.api_url == "http://127.0.0.1:3000";
+ assert config.retries == 3
+ assert config.has_jwt is False
+ assert config.heartbeat_interval == timedelta(seconds=5)
+
+ def test_every_field_round_trips(self):
+ """Test that each configured field is readable back unchanged."""
+ config = HttpConfig(
+ api_url="http://127.0.0.1:3001";,
+ retries=5,
+ jwt="a-token",
+ heartbeat_interval=timedelta(seconds=15),
+ )
+
+ assert config.api_url == "http://127.0.0.1:3001";
+ assert config.retries == 5
+ assert config.has_jwt is True
+ assert config.heartbeat_interval == timedelta(seconds=15)
+
+ def test_arguments_are_keyword_only(self):
+ """Test that the API URL cannot be passed positionally."""
+ with pytest.raises(TypeError):
+ # pyrefly: ignore # bad-argument-count
+ HttpConfig("http://127.0.0.1:3000";)
+
+ def test_repr_hides_the_jwt(self):
+ """Test that the JWT does not leak through repr but still parses as
Python."""
+ config = HttpConfig(jwt="a-secret-token")
+
+ printed = repr(config)
+
+ assert "a-secret-token" not in printed
+ ast.parse(printed)
+
+ def test_repr_shows_every_field_as_python(self):
Review Comment:
nit: this skips `heartbeat_interval`, the one field whose repr is
format-fragile, and `ast.parse` won't catch a regression there. add
`heartbeat_interval=timedelta(seconds=15)` to the config below, then assert it
- asserting alone goes red against the 5s default.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -0,0 +1,190 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests for the HTTP client configuration surface.
+
+`HttpConfig` mirrors the Rust SDK's `HttpClientConfig` the same way
+`TcpConfig`/`QuicConfig` do, so most of these assert that a value set from
+Python survives to the getters and that unset fields fall back to the Rust
+defaults. HTTP is stateless per-request, so unlike TCP/QUIC there is no
+`AutoLogin` or reconnection policy to configure.
+"""
+
+import ast
+from datetime import timedelta
+
+import pytest
+
+from apache_iggy import Consumer, HttpConfig, IggyClient, PollingStrategy
+from apache_iggy import SendMessage as Message
+
+from .utils import get_http_server_config, wait_for_ping
+
+
[email protected]
+class TestHttpConfig:
+ """Test the transport configuration."""
+
+ def test_defaults_match_the_rust_sdk(self):
+ """Test that an unconfigured transport matches the Rust SDK
defaults."""
+ config = HttpConfig()
+
+ assert config.api_url == "http://127.0.0.1:3000";
+ assert config.retries == 3
+ assert config.has_jwt is False
+ assert config.heartbeat_interval == timedelta(seconds=5)
+
+ def test_every_field_round_trips(self):
+ """Test that each configured field is readable back unchanged."""
+ config = HttpConfig(
+ api_url="http://127.0.0.1:3001";,
+ retries=5,
+ jwt="a-token",
+ heartbeat_interval=timedelta(seconds=15),
+ )
+
+ assert config.api_url == "http://127.0.0.1:3001";
+ assert config.retries == 5
+ assert config.has_jwt is True
+ assert config.heartbeat_interval == timedelta(seconds=15)
+
+ def test_arguments_are_keyword_only(self):
+ """Test that the API URL cannot be passed positionally."""
+ with pytest.raises(TypeError):
+ # pyrefly: ignore # bad-argument-count
+ HttpConfig("http://127.0.0.1:3000";)
+
+ def test_repr_hides_the_jwt(self):
+ """Test that the JWT does not leak through repr but still parses as
Python."""
+ config = HttpConfig(jwt="a-secret-token")
+
+ printed = repr(config)
+
+ assert "a-secret-token" not in printed
+ ast.parse(printed)
+
+ def test_repr_shows_every_field_as_python(self):
+ """Test that repr covers the configured fields and parses as Python."""
+ config = HttpConfig(api_url="http://127.0.0.1:3001";, retries=5)
+
+ printed = repr(config)
+
+ assert 'api_url="http://127.0.0.1:3001";' in printed
+ assert "retries=5" in printed
+ ast.parse(printed)
+
+ @pytest.mark.parametrize(
+ "invalid_url",
+ ["", "not-a-url", "http://127.0.0.1:0";],
+ )
+ def test_invalid_api_url_is_rejected(self, invalid_url: str):
+ """Test that a malformed API URL fails at construction, not at
connect."""
+ with pytest.raises(ValueError):
+ HttpConfig(api_url=invalid_url)
+
+ @pytest.mark.parametrize("out_of_range", [-1, 2**32])
+ def test_out_of_range_retries_is_rejected(self, out_of_range: int):
+ """Test that a retry count outside the wire range names the argument.
+
+ The conversion pyo3 does on its own raises OverflowError, which is not
a
+ ValueError and so escapes the handler a caller wraps construction in.
+ """
+ with pytest.raises(ValueError, match="retries"):
+ HttpConfig(retries=out_of_range)
+
+ def test_negative_heartbeat_interval_is_rejected(self):
+ """Test that a negative heartbeat interval fails at construction."""
+ with pytest.raises(ValueError, match="negative"):
+ HttpConfig(heartbeat_interval=timedelta(seconds=-3))
+
+ def test_zero_heartbeat_interval_is_rejected(self):
+ """Test that a zero heartbeat interval fails at construction.
+
+ Nothing downstream reads zero as "disabled"; it heartbeats in a
+ continuous loop for as long as the client lives.
+ """
+ with pytest.raises(ValueError, match="zero"):
+ HttpConfig(heartbeat_interval=timedelta(0))
+
+
[email protected]
+class TestHttpClientConstruction:
+ """Test that `IggyClient(...)` accepts an `HttpConfig`."""
+
+ def test_accepts_a_config(self):
Review Comment:
nit: nothing here pins the transport - both assert only `is not None`, and
the integration test would pass with a TCP client too, since CI has 8090
listening. `IggyClient(HttpConfig(api_url="http://127.0.0.1:1";))` then `ping()`
must raise.
##########
foreign/python/src/config.rs:
##########
@@ -408,16 +409,127 @@ impl TcpConfig {
}
}
+/// Configuration for the HTTP transport, accepted by `IggyClient(...)`.
+///
+/// Every field is keyword-only and optional.
+#[gen_stub_pyclass]
+#[pyclass(from_py_object)]
+#[derive(Clone)]
+pub struct HttpConfig {
+ inner: Arc<RustHttpClientConfig>,
+}
+
+impl HttpConfig {
+ /// The configuration in the shape `HttpClient::create` expects.
+ pub(crate) fn client_config(&self) -> Arc<RustHttpClientConfig> {
+ self.inner.clone()
+ }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl HttpConfig {
+ /// Constructs an HTTP configuration.
+ ///
+ /// Args:
+ /// api_url: Base URL of the Iggy HTTP API. Defaults to
`http://127.0.0.1:3000`.
+ /// retries: Number of retries to perform on transient errors.
Defaults to 3.
+ /// jwt: JWT token for A2A (Agent-to-Agent) authentication. Defaults
to `None`.
+ /// heartbeat_interval: Interval of heartbeats sent by the client.
Defaults to 5 seconds.
+ ///
+ /// Raises:
+ /// ValueError: If `api_url` is not a valid URL, if `retries` is
outside the
+ /// range of an unsigned 32-bit integer, if a duration is
negative, or
+ /// if `heartbeat_interval` is zero.
+ #[new]
+ #[pyo3(signature = (*, api_url=None, retries=None, jwt=None,
heartbeat_interval=None))]
+ fn new(
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] api_url:
Option<String>,
+ #[gen_stub(override_type(type_repr = "builtins.int | None"))] retries:
Option<i64>,
+ #[gen_stub(override_type(type_repr = "builtins.str | None"))] jwt:
Option<String>,
+ #[gen_stub(override_type(type_repr = "datetime.timedelta | None",
imports=("datetime")))]
+ heartbeat_interval: Option<Py<PyDelta>>,
+ ) -> PyResult<Self> {
+ // The builder starts from `HttpClientConfig::default()`, and its
`build()`
+ // trims and validates the API URL whether or not one was set here.
+ let mut builder = HttpClientConfigBuilder::new();
+ if let Some(api_url) = api_url {
+ builder = builder.with_api_url(api_url);
+ }
+ if let Some(retries) = retries {
+ let retries = u32::try_from(retries).map_err(|_| {
+ PyValueError::new_err(format!("'retries' must be between 0 and
{}", u32::MAX))
+ })?;
+ builder = builder.with_retries(retries);
+ }
+ if let Some(jwt) = jwt {
+ builder = builder.with_jwt(jwt);
+ }
+ let mut inner = builder
Review Comment:
simplification: `HttpClientConfigBuilder::with_heartbeat_interval` already
takes the `NonZeroIggyDuration` that `reject_zero` returns, so this post-build
patch and the `mut` aren't needed. `TcpConfig` patches only because its builder
has no setter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
