CLI: add requires a run range (#47)

.github/workflows/cli-smoke.yml

@@ -80,7 +80,7 @@ jobs:
       - name: ccdb mktbl + add + vers
         run: |
           ccdb mktbl /test/smoke_dir/data_table -r 2 x y z
-          ccdb add /test/smoke_dir/data_table python/tests/test_table.txt
+          ccdb add /test/smoke_dir/data_table -r 0- python/tests/test_table.txt
           ccdb vers /test/smoke_dir/data_table
 
       - name: ccdb rm (empty table, then directory)

docs/cli.md

@@ -156,7 +156,8 @@ Assuming you are in interactive mode, the commands can be categorized:
 
 - `mkdir` — creates a directory
 - `mktbl` — creates a data table
-- `add` — adds data from a text file to a table (variation and run ranges are created automatically)
+- `add` — adds data from a text file to a table for a run range: `-r 1000-1999`, `-r 1000-` or `-r 0-` for every
+  run; the range is required (variation and run ranges are created automatically)
 - `deprecate` — hides a directory, table or variation from listings
 - `tag` — creates and removes tags
 - `prop set` — sets a property

python/ccdb/cmd/commands/add.py

@@ -33,6 +33,7 @@ def __init__(self, context):
         self.file_path = ""
         self.run_min = 0
         self.run_max = ccdb.INFINITE_RUN
+        self.run_range_given = False
         if self.context is None:
             self.variation = "default"
         else:
@@ -78,6 +79,11 @@ def execute(self, args):
             log.debug(LogFmt(" |- arguments validation {0}{1}{2}", self.theme.Fail, "failed", self.theme.Reset))
             raise ValueError("Arguments validation failed")
 
+        # the run range is required: before 2.5.1 a missing -r silently meant every run (issue #47)
+        if not self.run_range_given:
+            raise ValueError("add needs a run range: -r <run_min>-<run_max>, -r <run_min>- for every run from run_min "
+                             "on, or -r 0- for every run")
+
         # correct paths
         self.table_path = self.context.prepare_path(self.raw_table_path)
         self.file_path = self.raw_file_path
@@ -155,6 +161,7 @@ def process_arguments(self, args):
 
                     # there is a result
                     (self.run_min, self.run_max, run_min_set, run_max_set) = result
+                    self.run_range_given = True
 
                     # check how the bounds were set
                     if not run_min_set:
@@ -235,10 +242,10 @@ def print_help(self):
 
     <variation> - variation name 
 
-    <run_min>-<run_max> - run range. 
+    <run_min>-<run_max> - run range, required.
         if one inputs '<run_min>-' this means <run_min>-<infinit run>
-        if one inputs '-<run_max>' this means <0>-<run_max> 
-        if one omits runrange at all. The data will be put as
+        if one inputs '-<run_max>' this means <0>-<run_max>
+        '-r 0-' puts the data to every run (what a missing -r meant before CCDB 2.5.1)
 
    file_to_import - file to import. It should be ccdb file format (see documentation or file format section) 
                     if file format is column of names and column of values add --name-value flag

python/tests/integ_test_cli_manager.py

@@ -285,14 +285,25 @@ def test_add_rm_assignment(self):
         tests_dir = os.path.dirname(os.path.realpath(__file__))
         test_file = os.path.join(tests_dir, "test_table.txt")
         print(test_file)
-        self.cli.process_command_line("add /test/test_vars/test_table " + test_file)
+        self.cli.process_command_line("add /test/test_vars/test_table -r 0- " + test_file)
         self.output.truncate(0)
         self.cli.process_command_line("vers /test/test_vars/test_table")
         text = str(self.output.getvalue())
         line = text.split("\n")[1]
         assignment_id = int(shlex.split(line)[0])
         self.cli.process_command_line("rm -f -a {0}".format(assignment_id))
 
+    def test_add_requires_run_range(self):
+        """add. Without -r the command refuses instead of putting the data to every run (#47)"""
+        from ccdb.model import Assignment
+        test_file = os.path.join(os.path.dirname(os.path.realpath(__file__)), "test_table.txt")
+        self.cli.process_command_line("ls")  # connects
+        before = self.cli.provider.session.query(Assignment).count()
+        with self.assertRaises(ValueError) as caught:
+            self.cli.process_command_line("add /test/test_vars/test_table " + test_file)
+        self.assertIn("-r 0-", str(caught.exception))
+        self.assertEqual(self.cli.provider.session.query(Assignment).count(), before)
+
     def test_add_with_run_range_assignment(self):
         """Regression tests for failing add with run range"""
         tests_dir = os.path.dirname(os.path.realpath(__file__))

python/tests/unit_test_dump_comments.py

@@ -70,7 +70,7 @@ def test_same_content(self):
 
         self.test_data_path = os.path.join(helper.ccdb_test_path, 'channel_mc_efficiency.list')
 
-        self.context.process_command_line("add /test/channel_mc_efficiency " + self.test_data_path)
+        self.context.process_command_line("add /test/channel_mc_efficiency -r 0- " + self.test_data_path)
 
         with helper.captured_output() as (out, err):
             self.context.process_command_line("dump /test/channel_mc_efficiency")

python/tests/unit_test_provider_shared_sets.py

@@ -712,7 +712,7 @@ def fail(mapper, connection, target):
         with self.subTest("cli: add and log"):
             path = self.new_fresh_db()
             cli = self.new_cli(path, log_sqlite=True)
-            self.run_cli(cli, "add {} {}".format(TABLE, os.path.join(TESTS_DIR, "test_table.txt")))
+            self.run_cli(cli, "add {} -r 0- {}".format(TABLE, os.path.join(TESTS_DIR, "test_table.txt")))
             new_id = self.query(path, "SELECT MAX(id) FROM assignments")[0][0]
             self.assertEqual(self.set_id_of_assignment(path, new_id), 4)
             self.assertEqual(self.query(path, 'SELECT "affectedIds" FROM logs ORDER BY id DESC LIMIT 1'),
@@ -725,7 +725,7 @@ def test_vers_l(self):
         path = self.new_fresh_db()
         cli = self.new_cli(path)
         for _ in range(2):      # two more sharers of seed set 4 (test_table.txt holds its bytes): 3 in all
-            self.run_cli(cli, "add {} {}".format(TABLE, os.path.join(TESTS_DIR, "test_table.txt")))
+            self.run_cli(cli, "add {} -r 0- {}".format(TABLE, os.path.join(TESTS_DIR, "test_table.txt")))
         new_ids = [row[0] for row in self.query(path, "SELECT id FROM assignments WHERE id > 5 ORDER BY id")]
         self.assertEqual(len(new_ids), 2)
 

python/tests/unit_test_units.py

@@ -95,7 +95,7 @@ def test_info_shows_units(self):
     def test_dump_header_shows_units(self):
         self.run_cmd("mktbl /test/units_t -r 2 gain=double:MeV n=int 'width=double:counts per s'")
         data_file = self.write_file("in.txt", "#calibration of the day\n1.5 3 0.1\n2.5 4 0.2\n")
-        self.run_cmd("add /test/units_t " + data_file)
+        self.run_cmd("add /test/units_t -r 0- " + data_file)
 
         dumped = self.run_cmd("dump /test/units_t")
         lines = [ln.strip() for ln in dumped.strip().splitlines()]
@@ -111,15 +111,15 @@ def test_dump_header_shows_units(self):
         self.assertEqual(parse_units_meta(dom.metas["units"]), {"gain": "MeV", "width": "counts per s"})
 
         # adding the dump back does not grow the assignment comment with the units line
-        self.run_cmd("add /test/units_t " + dump_file)
+        self.run_cmd("add /test/units_t -r 0- " + dump_file)
         dumped_again = self.run_cmd("dump /test/units_t")
         self.assertEqual(dumped_again, dumped)
 
     def test_dump_without_units_has_no_meta_line(self):
         """A table without units dumps exactly as before units existed"""
         self.run_cmd("mktbl /test/plain_t -r 2 x y=int")
         data_file = self.write_file("plain.txt", "#plain\n1.5 3\n2.5 4\n")
-        self.run_cmd("add /test/plain_t " + data_file)
+        self.run_cmd("add /test/plain_t -r 0- " + data_file)
         dumped = self.run_cmd("dump /test/plain_t")
         self.assertNotIn("#meta", dumped)
         lines = [ln.strip() for ln in dumped.strip().splitlines()]