github-actions[bot] commented on code in PR #4490:
URL: https://github.com/apache/iggy/pull/4490#discussion_r4247882744


##########
foreign/python/src/cluster.rs:
##########
@@ -0,0 +1,220 @@
+// 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.
+
+use iggy::prelude::{
+    ClusterMetadata as RustClusterMetadata, ClusterNode as RustClusterNode,
+    ClusterNodeRole as RustClusterNodeRole, ClusterNodeStatus as 
RustClusterNodeStatus,
+    TransportEndpoints as RustTransportEndpoints,
+};
+use pyo3::prelude::*;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_enum, 
gen_stub_pymethods};
+
+/// The role of a node within the cluster.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeRole {
+    /// Primary/Leader node - handles all writes.
+    Leader,
+    /// Follower/Secondary node - read replica.
+    Follower,
+}
+
+impl From<RustClusterNodeRole> for ClusterNodeRole {
+    fn from(role: RustClusterNodeRole) -> Self {
+        match role {
+            RustClusterNodeRole::Leader => ClusterNodeRole::Leader,
+            RustClusterNodeRole::Follower => ClusterNodeRole::Follower,
+        }
+    }
+}
+
+/// The current status of a node.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeStatus {
+    /// Node is healthy and responsive.
+    Healthy,
+    /// Node is starting up.
+    Starting,
+    /// Node is shutting down.
+    Stopping,
+    /// Node is unreachable.
+    Unreachable,
+    /// Node is in maintenance mode.
+    Maintenance,
+    /// Node is unknown.
+    Unknown,
+}
+
+impl From<RustClusterNodeStatus> for ClusterNodeStatus {
+    fn from(status: RustClusterNodeStatus) -> Self {
+        match status {
+            RustClusterNodeStatus::Healthy => ClusterNodeStatus::Healthy,
+            RustClusterNodeStatus::Starting => ClusterNodeStatus::Starting,
+            RustClusterNodeStatus::Stopping => ClusterNodeStatus::Stopping,
+            RustClusterNodeStatus::Unreachable => 
ClusterNodeStatus::Unreachable,
+            RustClusterNodeStatus::Maintenance => 
ClusterNodeStatus::Maintenance,
+            RustClusterNodeStatus::Unknown => ClusterNodeStatus::Unknown,
+        }
+    }
+}
+
+/// Transport endpoints exposed by a cluster node.
+///
+/// A port of 0 means the node does not advertise that transport.
+#[gen_stub_pyclass]
+#[pyclass(frozen)]
+pub struct TransportEndpoints {
+    /// TCP port.
+    #[pyo3(get)]
+    pub tcp: u16,
+    /// QUIC port.
+    #[pyo3(get)]
+    pub quic: u16,
+    /// HTTP port.
+    #[pyo3(get)]
+    pub http: u16,
+    /// WebSocket port.
+    #[pyo3(get)]
+    pub websocket: u16,
+}
+
+impl From<&RustTransportEndpoints> for TransportEndpoints {
+    fn from(endpoints: &RustTransportEndpoints) -> Self {
+        Self {
+            tcp: endpoints.tcp,
+            quic: endpoints.quic,
+            http: endpoints.http,
+            websocket: endpoints.websocket,
+        }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl TransportEndpoints {
+    fn __repr__(&self) -> String {
+        format!(
+            "TransportEndpoints(tcp={}, quic={}, http={}, websocket={})",
+            self.tcp, self.quic, self.http, self.websocket
+        )
+    }
+}
+
+/// A node in the cluster with its transport endpoints, role, and status.
+#[gen_stub_pyclass]
+#[pyclass]
+pub struct ClusterNode {
+    pub(crate) inner: RustClusterNode,
+}
+
+impl From<RustClusterNode> for ClusterNode {
+    fn from(node: RustClusterNode) -> Self {
+        Self { inner: node }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl ClusterNode {
+    /// The name of the node.
+    #[getter]
+    pub fn name(&self) -> String {
+        self.inner.name.clone()
+    }
+
+    /// The address clients should use to reach the node.

Review Comment:
   nit: `ClusterNode.ip` holds a bare host without brackets, so joining it with 
a port breaks for an IPv6 address. Document the bare-host form and state that 
IPv6 needs brackets before the port.



##########
foreign/python/tests/test_cluster.py:
##########
@@ -0,0 +1,243 @@
+# 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.
+
+import contextlib
+import json
+import threading
+from collections.abc import Iterator
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+
+import pytest
+
+from apache_iggy import ClusterNodeRole, ClusterNodeStatus, HttpConfig, 
IggyClient
+
+from .utils import (
+    get_server_config,
+    login_fresh_client,
+    unique_credentials,
+    wait_for_server,
+)
+
+MAX_PORT = 65535
+
+# A node's role and status as the HTTP API spells them, then the Python values
+# they must become. A server without clustering only reports a healthy leader,
+# so the TCP tests below never see the other values.
+REPORTED_NODES = [
+    ("leader", "healthy", ClusterNodeRole.Leader, ClusterNodeStatus.Healthy),
+    ("follower", "starting", ClusterNodeRole.Follower, 
ClusterNodeStatus.Starting),
+    ("follower", "stopping", ClusterNodeRole.Follower, 
ClusterNodeStatus.Stopping),
+    (
+        "follower",
+        "unreachable",
+        ClusterNodeRole.Follower,
+        ClusterNodeStatus.Unreachable,
+    ),
+    (
+        "follower",
+        "maintenance",
+        ClusterNodeRole.Follower,
+        ClusterNodeStatus.Maintenance,
+    ),
+    ("follower", "unknown", ClusterNodeRole.Follower, 
ClusterNodeStatus.Unknown),
+]
+
+
[email protected]
+def serve_cluster_metadata() -> Iterator[int]:
+    """Serve a multi-node `GET /cluster/metadata` on a free local port.
+
+    `/ping` is answered too, because connecting an HTTP client starts a 
heartbeat.
+    """
+    metadata = {
+        "name": "stub-cluster",
+        "nodes": [
+            {
+                "name": f"node-{index}",
+                "ip": f"10.0.0.{index}",
+                "endpoints": {
+                    "tcp": 8090 + index,
+                    "quic": 8080 + index,
+                    "http": 3000 + index,
+                    "websocket": 8092 + index,
+                },
+                "role": role,
+                "status": status,
+            }
+            for index, (role, status, _, _) in enumerate(REPORTED_NODES, 
start=1)
+        ],
+    }
+    responses = {
+        "/cluster/metadata": ("application/json", 
json.dumps(metadata).encode()),
+        "/ping": ("text/plain", b"pong"),
+    }
+
+    class Handler(BaseHTTPRequestHandler):
+        def do_GET(self):
+            if self.path not in responses:
+                self.send_error(404)
+                return
+            content_type, body = responses[self.path]
+            self.send_response(200)
+            self.send_header("Content-Type", content_type)
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, format, *args):  # noqa: ARG002
+            """Keep the test output free of per-request log lines."""
+
+    server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
+    thread = threading.Thread(target=server.serve_forever, daemon=True)
+    thread.start()
+    try:
+        yield server.server_address[1]
+    finally:
+        server.shutdown()
+        server.server_close()
+        thread.join()
+
+
+class TestClusterMetadata:
+    """Test cluster metadata retrieval."""
+
+    @pytest.mark.asyncio
+    async def test_get_cluster_metadata(self, iggy_client: IggyClient):
+        """A server running without clustering reports itself as the only node,
+        the healthy leader."""
+        metadata = await iggy_client.get_cluster_metadata()
+
+        assert metadata.name
+        nodes = metadata.nodes
+        assert len(nodes) == 1
+
+        node = nodes[0]
+        assert node.name
+        assert node.ip
+        assert node.role == ClusterNodeRole.Leader
+        assert node.status == ClusterNodeStatus.Healthy
+
+        endpoints = node.endpoints
+        # The connection under test is TCP, so the node must advertise it. The
+        # port is only bounded because the test server may be reached through a
+        # published port that differs from the one it listens on.
+        assert 0 < endpoints.tcp <= MAX_PORT
+        for port in (endpoints.quic, endpoints.http, endpoints.websocket):

Review Comment:
   simplification: The loop and the `<​= MAX_PORT` bound cannot fail, because 
the endpoint fields are `u16` values. Keep `assert endpoints.tcp > 0` at line 
138, then delete `MAX_PORT` and the loop.



##########
foreign/python/src/cluster.rs:
##########
@@ -0,0 +1,220 @@
+// 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.
+
+use iggy::prelude::{
+    ClusterMetadata as RustClusterMetadata, ClusterNode as RustClusterNode,
+    ClusterNodeRole as RustClusterNodeRole, ClusterNodeStatus as 
RustClusterNodeStatus,
+    TransportEndpoints as RustTransportEndpoints,
+};
+use pyo3::prelude::*;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_enum, 
gen_stub_pymethods};
+
+/// The role of a node within the cluster.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeRole {
+    /// Primary/Leader node - handles all writes.
+    Leader,
+    /// Follower/Secondary node - read replica.
+    Follower,
+}
+
+impl From<RustClusterNodeRole> for ClusterNodeRole {
+    fn from(role: RustClusterNodeRole) -> Self {
+        match role {
+            RustClusterNodeRole::Leader => ClusterNodeRole::Leader,
+            RustClusterNodeRole::Follower => ClusterNodeRole::Follower,
+        }
+    }
+}
+
+/// The current status of a node.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeStatus {
+    /// Node is healthy and responsive.
+    Healthy,
+    /// Node is starting up.
+    Starting,
+    /// Node is shutting down.
+    Stopping,
+    /// Node is unreachable.
+    Unreachable,
+    /// Node is in maintenance mode.
+    Maintenance,
+    /// Node is unknown.
+    Unknown,
+}
+
+impl From<RustClusterNodeStatus> for ClusterNodeStatus {
+    fn from(status: RustClusterNodeStatus) -> Self {
+        match status {
+            RustClusterNodeStatus::Healthy => ClusterNodeStatus::Healthy,
+            RustClusterNodeStatus::Starting => ClusterNodeStatus::Starting,
+            RustClusterNodeStatus::Stopping => ClusterNodeStatus::Stopping,
+            RustClusterNodeStatus::Unreachable => 
ClusterNodeStatus::Unreachable,
+            RustClusterNodeStatus::Maintenance => 
ClusterNodeStatus::Maintenance,
+            RustClusterNodeStatus::Unknown => ClusterNodeStatus::Unknown,
+        }
+    }
+}
+
+/// Transport endpoints exposed by a cluster node.
+///
+/// A port of 0 means the node does not advertise that transport.
+#[gen_stub_pyclass]
+#[pyclass(frozen)]
+pub struct TransportEndpoints {
+    /// TCP port.
+    #[pyo3(get)]
+    pub tcp: u16,
+    /// QUIC port.
+    #[pyo3(get)]
+    pub quic: u16,
+    /// HTTP port.
+    #[pyo3(get)]
+    pub http: u16,
+    /// WebSocket port.
+    #[pyo3(get)]
+    pub websocket: u16,
+}
+
+impl From<&RustTransportEndpoints> for TransportEndpoints {
+    fn from(endpoints: &RustTransportEndpoints) -> Self {
+        Self {
+            tcp: endpoints.tcp,
+            quic: endpoints.quic,
+            http: endpoints.http,
+            websocket: endpoints.websocket,
+        }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl TransportEndpoints {
+    fn __repr__(&self) -> String {
+        format!(
+            "TransportEndpoints(tcp={}, quic={}, http={}, websocket={})",
+            self.tcp, self.quic, self.http, self.websocket
+        )
+    }
+}
+
+/// A node in the cluster with its transport endpoints, role, and status.
+#[gen_stub_pyclass]
+#[pyclass]
+pub struct ClusterNode {
+    pub(crate) inner: RustClusterNode,
+}
+
+impl From<RustClusterNode> for ClusterNode {
+    fn from(node: RustClusterNode) -> Self {
+        Self { inner: node }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl ClusterNode {
+    /// The name of the node.
+    #[getter]
+    pub fn name(&self) -> String {
+        self.inner.name.clone()
+    }
+
+    /// The address clients should use to reach the node.
+    #[getter]
+    pub fn ip(&self) -> String {
+        self.inner.ip.clone()
+    }
+
+    /// The transport endpoints of the node.
+    #[getter]
+    pub fn endpoints(&self) -> TransportEndpoints {
+        TransportEndpoints::from(&self.inner.endpoints)
+    }
+
+    /// The role of the node in the cluster.
+    #[getter]
+    pub fn role(&self) -> ClusterNodeRole {
+        ClusterNodeRole::from(self.inner.role)
+    }
+
+    /// The current status of the node.
+    #[getter]
+    pub fn status(&self) -> ClusterNodeStatus {
+        ClusterNodeStatus::from(self.inner.status)
+    }
+
+    fn __repr__(&self) -> String {
+        format!(
+            "ClusterNode(name='{}', ip='{}', endpoints={}, 
role=ClusterNodeRole.{:?}, status=ClusterNodeStatus.{:?})",
+            self.inner.name,
+            self.inner.ip,
+            TransportEndpoints::from(&self.inner.endpoints).__repr__(),
+            ClusterNodeRole::from(self.inner.role),
+            ClusterNodeStatus::from(self.inner.status),
+        )
+    }
+}
+
+/// Metadata of all nodes in the cluster.
+///
+/// A server running without clustering reports itself as the only node.
+#[gen_stub_pyclass]
+#[pyclass]
+pub struct ClusterMetadata {
+    pub(crate) inner: RustClusterMetadata,
+}
+
+impl From<RustClusterMetadata> for ClusterMetadata {
+    fn from(metadata: RustClusterMetadata) -> Self {
+        Self { inner: metadata }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl ClusterMetadata {
+    /// The name of the cluster.
+    #[getter]
+    pub fn name(&self) -> String {
+        self.inner.name.clone()
+    }
+
+    /// The list of all nodes in the cluster.
+    #[getter]
+    pub fn nodes(&self) -> Vec<ClusterNode> {

Review Comment:
   nit: `ClusterMetadata.nodes` builds a new list of new node objects on every 
access, but its docstring does not say so. Document the rebuild the way 
`Stats.cache_metrics` does at `foreign/python/src/stats.rs:293`, so callers can 
cache it.



##########
foreign/python/src/cluster.rs:
##########
@@ -0,0 +1,220 @@
+// 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.
+
+use iggy::prelude::{
+    ClusterMetadata as RustClusterMetadata, ClusterNode as RustClusterNode,
+    ClusterNodeRole as RustClusterNodeRole, ClusterNodeStatus as 
RustClusterNodeStatus,
+    TransportEndpoints as RustTransportEndpoints,
+};
+use pyo3::prelude::*;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_enum, 
gen_stub_pymethods};
+
+/// The role of a node within the cluster.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeRole {
+    /// Primary/Leader node - handles all writes.
+    Leader,
+    /// Follower/Secondary node - read replica.
+    Follower,
+}
+
+impl From<RustClusterNodeRole> for ClusterNodeRole {
+    fn from(role: RustClusterNodeRole) -> Self {
+        match role {
+            RustClusterNodeRole::Leader => ClusterNodeRole::Leader,
+            RustClusterNodeRole::Follower => ClusterNodeRole::Follower,
+        }
+    }
+}
+
+/// The current status of a node.
+#[gen_stub_pyclass_enum]
+#[pyclass(eq, frozen, hash, skip_from_py_object)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ClusterNodeStatus {
+    /// Node is healthy and responsive.
+    Healthy,
+    /// Node is starting up.
+    Starting,
+    /// Node is shutting down.
+    Stopping,
+    /// Node is unreachable.
+    Unreachable,
+    /// Node is in maintenance mode.
+    Maintenance,
+    /// Node is unknown.
+    Unknown,
+}
+
+impl From<RustClusterNodeStatus> for ClusterNodeStatus {
+    fn from(status: RustClusterNodeStatus) -> Self {
+        match status {
+            RustClusterNodeStatus::Healthy => ClusterNodeStatus::Healthy,
+            RustClusterNodeStatus::Starting => ClusterNodeStatus::Starting,
+            RustClusterNodeStatus::Stopping => ClusterNodeStatus::Stopping,
+            RustClusterNodeStatus::Unreachable => 
ClusterNodeStatus::Unreachable,
+            RustClusterNodeStatus::Maintenance => 
ClusterNodeStatus::Maintenance,
+            RustClusterNodeStatus::Unknown => ClusterNodeStatus::Unknown,
+        }
+    }
+}
+
+/// Transport endpoints exposed by a cluster node.
+///
+/// A port of 0 means the node does not advertise that transport.
+#[gen_stub_pyclass]
+#[pyclass(frozen)]
+pub struct TransportEndpoints {
+    /// TCP port.
+    #[pyo3(get)]
+    pub tcp: u16,
+    /// QUIC port.
+    #[pyo3(get)]
+    pub quic: u16,
+    /// HTTP port.
+    #[pyo3(get)]
+    pub http: u16,
+    /// WebSocket port.
+    #[pyo3(get)]
+    pub websocket: u16,
+}
+
+impl From<&RustTransportEndpoints> for TransportEndpoints {
+    fn from(endpoints: &RustTransportEndpoints) -> Self {
+        Self {
+            tcp: endpoints.tcp,
+            quic: endpoints.quic,
+            http: endpoints.http,
+            websocket: endpoints.websocket,
+        }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl TransportEndpoints {
+    fn __repr__(&self) -> String {
+        format!(
+            "TransportEndpoints(tcp={}, quic={}, http={}, websocket={})",
+            self.tcp, self.quic, self.http, self.websocket
+        )
+    }
+}
+
+/// A node in the cluster with its transport endpoints, role, and status.
+#[gen_stub_pyclass]
+#[pyclass]
+pub struct ClusterNode {
+    pub(crate) inner: RustClusterNode,
+}
+
+impl From<RustClusterNode> for ClusterNode {
+    fn from(node: RustClusterNode) -> Self {
+        Self { inner: node }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl ClusterNode {
+    /// The name of the node.
+    #[getter]
+    pub fn name(&self) -> String {
+        self.inner.name.clone()
+    }
+
+    /// The address clients should use to reach the node.
+    #[getter]
+    pub fn ip(&self) -> String {
+        self.inner.ip.clone()
+    }
+
+    /// The transport endpoints of the node.
+    #[getter]
+    pub fn endpoints(&self) -> TransportEndpoints {
+        TransportEndpoints::from(&self.inner.endpoints)
+    }
+
+    /// The role of the node in the cluster.
+    #[getter]
+    pub fn role(&self) -> ClusterNodeRole {
+        ClusterNodeRole::from(self.inner.role)
+    }
+
+    /// The current status of the node.
+    #[getter]
+    pub fn status(&self) -> ClusterNodeStatus {
+        ClusterNodeStatus::from(self.inner.status)
+    }
+
+    fn __repr__(&self) -> String {
+        format!(
+            "ClusterNode(name='{}', ip='{}', endpoints={}, 
role=ClusterNodeRole.{:?}, status=ClusterNodeStatus.{:?})",
+            self.inner.name,
+            self.inner.ip,
+            TransportEndpoints::from(&self.inner.endpoints).__repr__(),
+            ClusterNodeRole::from(self.inner.role),
+            ClusterNodeStatus::from(self.inner.status),
+        )
+    }
+}
+
+/// Metadata of all nodes in the cluster.
+///
+/// A server running without clustering reports itself as the only node.
+#[gen_stub_pyclass]
+#[pyclass]
+pub struct ClusterMetadata {
+    pub(crate) inner: RustClusterMetadata,
+}
+
+impl From<RustClusterMetadata> for ClusterMetadata {
+    fn from(metadata: RustClusterMetadata) -> Self {
+        Self { inner: metadata }
+    }
+}
+
+#[gen_stub_pymethods]
+#[pymethods]
+impl ClusterMetadata {
+    /// The name of the cluster.
+    #[getter]
+    pub fn name(&self) -> String {
+        self.inner.name.clone()
+    }
+
+    /// The list of all nodes in the cluster.
+    #[getter]
+    pub fn nodes(&self) -> Vec<ClusterNode> {
+        self.inner
+            .nodes
+            .iter()
+            .cloned()
+            .map(ClusterNode::from)
+            .collect()
+    }
+
+    fn __repr__(&self) -> String {

Review Comment:
   nit: `ClusterMetadata.__repr__` prints the node count as `nodes=1`, reusing 
the name of the property that returns the list. Rename it to `nodes_count`, as 
`Stats.__repr__` does with `streams_count`, and update the two test assertions 
that match on it.



##########
foreign/python/tests/test_cluster.py:
##########
@@ -0,0 +1,243 @@
+# 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.
+
+import contextlib
+import json
+import threading
+from collections.abc import Iterator
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+
+import pytest
+
+from apache_iggy import ClusterNodeRole, ClusterNodeStatus, HttpConfig, 
IggyClient
+
+from .utils import (
+    get_server_config,
+    login_fresh_client,
+    unique_credentials,
+    wait_for_server,
+)
+
+MAX_PORT = 65535
+
+# A node's role and status as the HTTP API spells them, then the Python values
+# they must become. A server without clustering only reports a healthy leader,
+# so the TCP tests below never see the other values.
+REPORTED_NODES = [
+    ("leader", "healthy", ClusterNodeRole.Leader, ClusterNodeStatus.Healthy),
+    ("follower", "starting", ClusterNodeRole.Follower, 
ClusterNodeStatus.Starting),
+    ("follower", "stopping", ClusterNodeRole.Follower, 
ClusterNodeStatus.Stopping),
+    (
+        "follower",
+        "unreachable",
+        ClusterNodeRole.Follower,
+        ClusterNodeStatus.Unreachable,
+    ),
+    (
+        "follower",
+        "maintenance",
+        ClusterNodeRole.Follower,
+        ClusterNodeStatus.Maintenance,
+    ),
+    ("follower", "unknown", ClusterNodeRole.Follower, 
ClusterNodeStatus.Unknown),
+]
+
+
[email protected]
+def serve_cluster_metadata() -> Iterator[int]:
+    """Serve a multi-node `GET /cluster/metadata` on a free local port.
+
+    `/ping` is answered too, because connecting an HTTP client starts a 
heartbeat.
+    """
+    metadata = {
+        "name": "stub-cluster",
+        "nodes": [
+            {
+                "name": f"node-{index}",
+                "ip": f"10.0.0.{index}",
+                "endpoints": {
+                    "tcp": 8090 + index,
+                    "quic": 8080 + index,
+                    "http": 3000 + index,
+                    "websocket": 8092 + index,
+                },
+                "role": role,
+                "status": status,
+            }
+            for index, (role, status, _, _) in enumerate(REPORTED_NODES, 
start=1)
+        ],
+    }
+    responses = {
+        "/cluster/metadata": ("application/json", 
json.dumps(metadata).encode()),
+        "/ping": ("text/plain", b"pong"),
+    }
+
+    class Handler(BaseHTTPRequestHandler):
+        def do_GET(self):
+            if self.path not in responses:
+                self.send_error(404)
+                return
+            content_type, body = responses[self.path]
+            self.send_response(200)
+            self.send_header("Content-Type", content_type)
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, format, *args):  # noqa: ARG002
+            """Keep the test output free of per-request log lines."""
+
+    server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
+    thread = threading.Thread(target=server.serve_forever, daemon=True)
+    thread.start()
+    try:
+        yield server.server_address[1]
+    finally:
+        server.shutdown()
+        server.server_close()
+        thread.join()
+
+
+class TestClusterMetadata:
+    """Test cluster metadata retrieval."""
+
+    @pytest.mark.asyncio
+    async def test_get_cluster_metadata(self, iggy_client: IggyClient):
+        """A server running without clustering reports itself as the only node,
+        the healthy leader."""
+        metadata = await iggy_client.get_cluster_metadata()
+
+        assert metadata.name
+        nodes = metadata.nodes
+        assert len(nodes) == 1
+
+        node = nodes[0]
+        assert node.name
+        assert node.ip
+        assert node.role == ClusterNodeRole.Leader
+        assert node.status == ClusterNodeStatus.Healthy
+
+        endpoints = node.endpoints
+        # The connection under test is TCP, so the node must advertise it. The
+        # port is only bounded because the test server may be reached through a
+        # published port that differs from the one it listens on.
+        assert 0 < endpoints.tcp <= MAX_PORT
+        for port in (endpoints.quic, endpoints.http, endpoints.websocket):
+            assert 0 <= port <= MAX_PORT
+
+        assert f"name='{metadata.name}'" in repr(metadata)
+        assert "nodes=1" in repr(metadata)
+        assert f"name='{node.name}'" in repr(node)
+        assert "role=ClusterNodeRole.Leader" in repr(node)
+        assert "status=ClusterNodeStatus.Healthy" in repr(node)
+        assert f"tcp={endpoints.tcp}" in repr(endpoints)
+
+    @pytest.mark.unit
+    def test_role_and_status_are_comparable_and_hashable(self):
+        """Roles and statuses compare by value and can key a dict."""
+        assert ClusterNodeRole.Leader == ClusterNodeRole.Leader

Review Comment:
   nit: This assertion and the hash one at line 154 use the same singleton on 
both sides, so neither detects a wrong result. Compare distinct members, or key 
a dict by the role as line 164 does for the status.



-- 
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]

Reply via email to