Chore: Address minor nitpicks and more from code review by CodeRabbit

Author: amotl

cratedb_toolkit/cluster/cli.py

@@ -106,6 +106,11 @@ def ping(ctx: click.Context, cluster_id: str, cluster_name: str):
     with handle_command_errors("ping cluster"):
         cluster = ManagedCluster(cluster_id=cluster_id, cluster_name=cluster_name).start()
         has_db_result = bool(cluster.query("SELECT 42 AS availability_check  -- ctk"))
+
+        # Ensure cluster.info is present.
+        if not cluster.info:
+            raise CroudException("Unable to retrieve cluster information")
+
         response = {
             "meta": cluster.info.meta,
             "cloud": cluster.info.ready,

cratedb_toolkit/cluster/croud.py

@@ -75,20 +75,28 @@ def with_cluster_id(self, cluster_id: str):
         self.cluster_id = cluster_id
         return self
 
+    def validate(self):
+        if not self.cluster_id:
+            raise ValueError("Cluster ID is not set")
+
     @property
     def home(self):
+        self.validate()
         return f"/api/v2/clusters/{self.cluster_id}/"
 
     @property
     def suspend(self):
+        self.validate()
         return f"/api/v2/clusters/{self.cluster_id}/suspend/"
 
     @property
     def jwt(self):
+        self.validate()
         return f"/api/v2/clusters/{self.cluster_id}/jwt/"
 
     @property
     def import_jobs(self):
+        self.validate()
         return f"/api/v2/clusters/{self.cluster_id}/import-jobs/"
 
 

cratedb_toolkit/testing/testcontainers/util.py

@@ -180,6 +180,11 @@ def __init__(self):
 
     @abstractmethod
     def setup(self):
+        """
+        Override this method to initialize self.container with a DockerContainer instance.
+        This method should create the container but NOT start it, as start() will be called
+        automatically after setup() completes successfully.
+        """
         raise NotImplementedError("Must be implemented by child class")
 
     def run_setup(self):

cratedb_toolkit/util/setting.py

@@ -49,8 +49,12 @@ def obtain_settings(specs: t.List[Setting], prog_name: str = None) -> t.Dict[str
     - Environment variable. Example: `export FOOBAR=bazqux`.
     - Environment variable prefix. Example: `export APPNAME_FOOBAR=bazqux`.
     """
+
     # Load environment variables from `.env` file.
-    init_dotenv()
+    try:
+        init_dotenv()
+    except Exception as ex:
+        logger.warning(f"Failed to load environment variables from .env file: {ex}")
 
     # Decode settings from command-line arguments or environment variables.
     prog_name = prog_name or sys.argv[0]
@@ -97,8 +101,10 @@ def check_mutual_exclusiveness(
         raise ValueError(message_none)
     if values.count(None) < len(values) - 1:
         if message_multiple is None:
+            # Collect and format the specified settings.
+            specified = [f"{param}={val}" for param, val in zip(parameter_names, values) if val is not None]
             message_multiple = (
-                f"The settings are mutually exclusive, but multiple of them have been specified. {guidance}"
+                f"The settings are mutually exclusive, but multiple were specified: {', '.join(specified)}. {guidance}"
             )
         raise ValueError(message_multiple)
     return values

doc/cluster/_address.md

@@ -1,5 +1,6 @@
 :::{rubric} Cluster address
 :::
+
 The program provides various options for addressing the database cluster
 in different situations, managed or not.
 

doc/cluster/backlog.md

@@ -95,4 +95,4 @@ a patch for some item, it will be very much welcome.
 - CLI: Shrink address URLs to single parameter `--cluster-url`
 - Changelog: Notify about breaking change with input address parameter names
 - Docs: Update guidelines about input address parameter preferences
-- Managed: Use `keyring` for caching the JWT token, and compensate token expiry
+- Managed: Use `keyring` for caching the JWT token, and compensate for token expiry

doc/cluster/python.md

@@ -2,7 +2,7 @@
 
 # CrateDB Cluster Python API
 
-The `cratedb_toolkit.ManagedCluster` class provides higher level API/SDK
+The `cratedb_toolkit.ManagedCluster` class provides the higher level API/SDK
 entrypoints to start/deploy/resume a database cluster, inquire information
 about it, and stop/suspend it again.
 

tests/cluster/test_cli.py

@@ -1,6 +1,31 @@
+import pytest
+from click import ClickException
 from click.testing import CliRunner
 
-from cratedb_toolkit.cluster.cli import cli
+from cratedb_toolkit.cluster.cli import cli, handle_command_errors
+from cratedb_toolkit.exception import CroudException
+
+
+def test_exception_handling_croud(caplog):
+    with pytest.raises(CroudException):
+        with handle_command_errors("test operation"):
+            raise CroudException("test error")
+    assert "Failed to test operation" in caplog.text
+    assert "test error" in caplog.text
+
+
+def test_exception_handling_click():
+    with pytest.raises(ClickException):
+        with handle_command_errors("test operation"):
+            raise ClickException("click error")
+
+
+def test_exception_handling_generic_to_system_exit(caplog):
+    with pytest.raises(SystemExit) as excinfo:
+        with handle_command_errors("test operation"):
+            raise ValueError("value error")
+    assert excinfo.value.code == 1
+    assert "Unexpected error on operation: test operation" in caplog.text
 
 
 def test_managed_cluster_info_default(cloud_environment):