diff --git a/.github/workflows/cli-smoke.yml b/.github/workflows/cli-smoke.yml index e396a5d6..e1f38e62 100644 --- a/.github/workflows/cli-smoke.yml +++ b/.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) diff --git a/docs/cli.md b/docs/cli.md index c21802cf..e6a843d9 100644 --- a/docs/cli.md +++ b/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 diff --git a/python/ccdb/cmd/commands/add.py b/python/ccdb/cmd/commands/add.py index d2e690b6..8e7b928b 100644 --- a/python/ccdb/cmd/commands/add.py +++ b/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 -, -r - 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 name - - - run range. + - - run range, required. if one inputs '-' this means - - if one inputs '-' this means <0>- - if one omits runrange at all. The data will be put as + if one inputs '-' this means <0>- + '-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 diff --git a/python/tests/integ_test_cli_manager.py b/python/tests/integ_test_cli_manager.py index 8ddc31b5..fac17f5d 100644 --- a/python/tests/integ_test_cli_manager.py +++ b/python/tests/integ_test_cli_manager.py @@ -285,7 +285,7 @@ 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()) @@ -293,6 +293,17 @@ def test_add_rm_assignment(self): 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__)) diff --git a/python/tests/unit_test_dump_comments.py b/python/tests/unit_test_dump_comments.py index da98cfc8..8ef30ca3 100644 --- a/python/tests/unit_test_dump_comments.py +++ b/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") diff --git a/python/tests/unit_test_provider_shared_sets.py b/python/tests/unit_test_provider_shared_sets.py index d64f5c3c..44264b4d 100644 --- a/python/tests/unit_test_provider_shared_sets.py +++ b/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) diff --git a/python/tests/unit_test_units.py b/python/tests/unit_test_units.py index b2497e29..0bcf5fef 100644 --- a/python/tests/unit_test_units.py +++ b/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,7 +111,7 @@ 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) @@ -119,7 +119,7 @@ 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()]