Better no connection handling

Author: DraTeots

.github/workflows/cli-smoke.yml

@@ -38,6 +38,27 @@ jobs:
       - name: "rcdb --version"
         run: rcdb --version
 
+      - name: "No connection fails cleanly (no traceback)"
+        env:
+          RCDB_CONNECTION: ""
+        run: |
+          # A command that needs the DB must exit non-zero with a clear message,
+          # not crash with a Python traceback.
+          set +e
+          OUT=$(rcdb select "event_count > 0" 2>&1)
+          CODE=$?
+          set -e
+          echo "$OUT"
+          echo "exit code: $CODE"
+          test "$CODE" -ne 0
+          echo "$OUT" | grep -q "No database connection provided"
+          if echo "$OUT" | grep -q "Traceback"; then
+            echo "ERROR: command crashed with a traceback instead of a clean error"
+            exit 1
+          fi
+          # `--help` must still work without a connection
+          rcdb select --help >/dev/null
+
       - name: "Create schema (db init)"
         run: rcdb db init --confirm
 

python/rcdb/app_context.py

@@ -14,11 +14,26 @@ def __init__(self, home, connection_str):
         self.connection_str = connection_str
 
     @property
-    def db(self):
+    def db(self) -> RCDBProvider:
         if not self._db_instance:
             self._db_instance = RCDBProvider(self.connection_str)
         return self._db_instance
 
+    def require_connected_db(self) -> RCDBProvider:
+        """Return the RCDBProvider, or fail with a clear CLI error if no connection was given.
+
+        Commands that cannot do anything without a database call this instead of
+        using ``db`` directly: a missing connection then produces a clean error
+        message rather than crashing later on a ``None`` session. Commands that can
+        run without a connection (e.g. ``web`` with ``--add-db``) keep using ``db``.
+        """
+        if not self.connection_str:
+            raise click.UsageError(
+                "No connection provided! Provide a DB connection string via the "
+                "-c/--connection option or the RCDB_CONNECTION environment variable."
+            )
+        return self.db
+
     def set_config(self, key, value):
         self.config[key] = value
         if self.verbose:

python/rcdb/cli/add.py

@@ -45,7 +45,7 @@ def add_type(context, name, value_type, description):
     Example:
       rcdb add type beam_current --type=float --description "Beam current in nA"
     """
-    db = context.db
+    db = context.require_connected_db()
     actual_type = TYPE_MAP[value_type.lower()]
 
     # Create or verify
@@ -69,7 +69,7 @@ def add_condition(context, run_number, condition_name, value, replace):
       rcdb add condition 1000 my_value 123.4
       rcdb add condition 1000 event_count 10000 --replace
     """
-    db = context.db
+    db = context.require_connected_db()
 
     # 1) Ensure run exists
     run = db.get_run(run_number)
@@ -115,7 +115,7 @@ def add_file(context, run_number, file_path, importance, overwrite, content):
       rcdb add file 1000 /path/to/coda_run.log
       rcdb add file 1000 /path/to/config.txt --importance=2 --overwrite
     """
-    db = context.db
+    db = context.require_connected_db()
     run = db.get_run(run_number)
     if not run:
         run = db.create_run(run_number)

python/rcdb/cli/app.py

@@ -44,10 +44,9 @@ def rcdb_cli(ctx, user_config, connection, config, verbose):
 
     # Create a rcdb_app_context object and remember it as the context object.  From
     # this point onwards other commands can refer to it by using the
-    # @pass_rcdb_context decorator.
-    if not connection:
-        print("(!)WARNING no connection provided! "
-              "Provide DB connection string via --connection/-c or RCDB_CONNECTION environment variable.")
+    # @pass_rcdb_context decorator. A connection is not validated here so that
+    # `--help` and connection-less commands stay clean; commands that need a
+    # database call context.require_connected_db() to get a clear error instead.
     ctx.obj = RcdbApplicationContext(os.path.abspath(user_config), connection)
     ctx.obj.verbose = verbose
     for key, value in config:

python/rcdb/cli/info.py

@@ -11,7 +11,7 @@ def info_command(context):
     """
     Shows various summary information about the RCDB database contents.
     """
-    db = context.db
+    db = context.require_connected_db()
 
     # Number of condition types
     cnd_type_count = db.session.query(ConditionType).count()

python/rcdb/cli/ls.py

@@ -1,6 +1,5 @@
 import click
 
-from rcdb.provider import RCDBProvider
 from .context import pass_rcdb_context
 
 
@@ -11,8 +10,7 @@
 def ls_command(context, search, is_long):
     """List conditions"""
 
-    db = context.db
-    assert isinstance(db, RCDBProvider)
+    db = context.require_connected_db()
     cnd_types = db.get_condition_types_by_name()
     names = sorted(cnd_types.keys())
     if search:

python/rcdb/cli/select.py

@@ -2,7 +2,6 @@
 import click
 
 from rcdb.app_context import parse_run_range
-from rcdb import RCDBProvider
 from rcdb.cli.context import pass_rcdb_context
 
 
@@ -37,14 +36,14 @@ def _process_sel_args(args):
 @pass_rcdb_context
 def select_command(rcdb_context, query, views_or_runs, is_dump_view, is_descending):
     """Select runs and get their values."""
-    assert isinstance(rcdb_context.db, RCDBProvider)
+    db = rcdb_context.require_connected_db()
     args = []
     if query is not None:
         args.append(str(query))
     args.extend([str(v) for v in views_or_runs])
     run_range_str, query, view = _process_sel_args(args)
 
-    run_periods = rcdb_context.db.get_run_periods()
+    run_periods = db.get_run_periods()
     run_min, run_max = parse_run_range(run_range_str, run_periods)
 
     if run_min is None:
@@ -60,7 +59,7 @@ def select_command(rcdb_context, query, views_or_runs, is_dump_view, is_descendi
 
     conditions_to_show = view.split()
 
-    values = rcdb_context.db.select_values([], query, run_min, run_max)
+    values = db.select_values([], query, run_min, run_max)
 
     if not is_dump_view: