Allow having a default port for gRPC URLs again

We allow client implementors to set a default port for gRPC URLs again,
but the `parse_grpc_uri` function and `BaseApiClient` class still don't
provide a default port unless explicitly set.

Signed-off-by: Leandro Lucarella <[email protected]>

RELEASE_NOTES.md

@@ -6,7 +6,7 @@
 
 ## Upgrading
 
-- Removes support for default ports in grpc URIs.  If a URI doesn't specify a default port, an exception is raised.
+- gRPC URLs don't have a default port anymore, unless a default is set via `ChannelOptions`. If you want to set a default port for URLs, please pass custom `ChannelOptions` as `defaults` to `parse_grpc_uri` or as `channel_defaults` to `BaseApiClient`.
 
 ## New Features
 

src/frequenz/client/base/channel.py

@@ -45,6 +45,9 @@ class SslOptions:
 class ChannelOptions:
     """Options for a gRPC channel."""
 
+    port: int | None = None
+    """The port number to connect to."""
+
     ssl: SslOptions = SslOptions()
     """SSL options for the channel."""
 
@@ -65,7 +68,8 @@ def parse_grpc_uri(
     A few things to consider about URI components:
 
     - If any other components are present in the URI, a [`ValueError`][] is raised.
-    - If the port is omitted, the `default_port` is used.
+    - If the port is omitted, the `default_port` is used unless it is `None`, in which
+      case a `ValueError` is raised
     - If a query parameter is passed many times, the last value is used.
     - Boolean query parameters can be specified with the following values
       (case-insensitive): `true`, `1`, `on`, `false`, `0`, `off`.
@@ -107,10 +111,14 @@ def parse_grpc_uri(
 
     options = _parse_query_params(uri, parsed_uri.query)
 
-    if not parsed_uri.port:
-        raise ValueError(f"The gRPC URI '{uri}' doesn't specify a port.")
+    if parsed_uri.port is None and defaults.port is None:
+        raise ValueError(
+            f"The gRPC URI '{uri}' doesn't specify a port and there is no default."
+        )
 
-    target = parsed_uri.netloc
+    target = (
+        parsed_uri.netloc if parsed_uri.port else f"{parsed_uri.netloc}:{defaults.port}"
+    )
 
     ssl = defaults.ssl.enabled if options.ssl is None else options.ssl
     if ssl:

tests/test_channel.py

@@ -24,7 +24,7 @@ class _ValidUrlTestCase:
     title: str
     uri: str
     expected_host: str
-    expected_port: int
+    expected_port: int | None
     expected_options: ChannelOptions
     defaults: ChannelOptions = ChannelOptions()
 
@@ -46,6 +46,21 @@ class _ValidUrlTestCase:
                 ),
             ),
         ),
+        _ValidUrlTestCase(
+            title="default with default port",
+            uri="grpc://localhost",
+            expected_host="localhost",
+            expected_port=9090,
+            expected_options=ChannelOptions(
+                ssl=SslOptions(
+                    enabled=True,
+                    root_certificates=None,
+                    private_key=None,
+                    certificate_chain=None,
+                ),
+            ),
+            defaults=ChannelOptions(port=9090),
+        ),
         _ValidUrlTestCase(
             title="default no SSL defaults",
             uri="grpc://localhost:2355",
@@ -265,3 +280,13 @@ def test_parse_uri_error(
     """Test parsing of invalid gRPC URIs for grpclib."""
     with pytest.raises(ValueError, match=error_msg):
         parse_grpc_uri(uri)
+
+
+def test_invalid_url_no_default_port() -> None:
+    """Test parsing of invalid gRPC URIs for grpclib."""
+    uri = "grpc://localhost"
+    with pytest.raises(
+        ValueError,
+        match=r"The gRPC URI 'grpc://localhost' doesn't specify a port and there is no default.",
+    ):
+        parse_grpc_uri(uri)