From 5b6865be3e33e8fe01bf26eaf8a5fe52fba4d7db Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 22 Sep 2023 14:34:36 -0500 Subject: [PATCH] Support quoted parameter list for MultiOption cli options (#8665) * allow multioption to be quoted * changelog * fix test * remove list format * fix tests * fix list object * review arg change * fix quotes * Update .changes/unreleased/Features-20230918-150855.yaml * add types * convert list to set in test * make mypy happy * mroe mypy happiness * more mypy happiness * last mypy change * add node to test --- .../unreleased/Features-20230918-150855.yaml | 6 + core/dbt/cli/flags.py | 2 +- core/dbt/cli/options.py | 27 ++-- tests/functional/cli/test_multioption.py | 142 ++++++++++++++++++ 4 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 .changes/unreleased/Features-20230918-150855.yaml create mode 100644 tests/functional/cli/test_multioption.py diff --git a/.changes/unreleased/Features-20230918-150855.yaml b/.changes/unreleased/Features-20230918-150855.yaml new file mode 100644 index 00000000000..b54b3447b7a --- /dev/null +++ b/.changes/unreleased/Features-20230918-150855.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Support quoted parameter list for MultiOption CLI options. +time: 2023-09-18T15:08:55.625412-05:00 +custom: + Author: emmyoop + Issue: "8598" diff --git a/core/dbt/cli/flags.py b/core/dbt/cli/flags.py index 0e54cfd5a8f..a9ea06edeed 100644 --- a/core/dbt/cli/flags.py +++ b/core/dbt/cli/flags.py @@ -52,7 +52,7 @@ def args_to_context(args: List[str]) -> Context: from dbt.cli.main import cli cli_ctx = cli.make_context(cli.name, args) - # Split args if they're a comma seperated string. + # Split args if they're a comma separated string. if len(args) == 1 and "," in args[0]: args = args[0].split(",") sub_command_name, sub_command, args = cli.resolve_command(cli_ctx, args) diff --git a/core/dbt/cli/options.py b/core/dbt/cli/options.py index 3a42dddda80..32d8e234b49 100644 --- a/core/dbt/cli/options.py +++ b/core/dbt/cli/options.py @@ -2,6 +2,7 @@ import inspect import typing as t from click import Context +from click.parser import OptionParser, ParsingState from dbt.cli.option_types import ChoiceTuple @@ -13,8 +14,9 @@ def __init__(self, *args, **kwargs): nargs = kwargs.pop("nargs", -1) assert nargs == -1, "nargs, if set, must be -1 not {}".format(nargs) super(MultiOption, self).__init__(*args, **kwargs) - self._previous_parser_process = None - self._eat_all_parser = None + # this makes mypy happy, setting these to None causes mypy failures + self._previous_parser_process = lambda *args, **kwargs: None + self._eat_all_parser = lambda *args, **kwargs: None # validate that multiple=True multiple = kwargs.pop("multiple", None) @@ -29,34 +31,35 @@ def __init__(self, *args, **kwargs): else: assert isinstance(option_type, ChoiceTuple), msg - def add_to_parser(self, parser, ctx): - def parser_process(value, state): + def add_to_parser(self, parser: OptionParser, ctx: Context): + def parser_process(value: str, state: ParsingState): # method to hook to the parser.process done = False - value = [value] + value_list = str.split(value, " ") if self.save_other_options: # grab everything up to the next option while state.rargs and not done: - for prefix in self._eat_all_parser.prefixes: + for prefix in self._eat_all_parser.prefixes: # type: ignore[attr-defined] if state.rargs[0].startswith(prefix): done = True if not done: - value.append(state.rargs.pop(0)) + value_list.append(state.rargs.pop(0)) else: # grab everything remaining - value += state.rargs + value_list += state.rargs state.rargs[:] = [] - value = tuple(value) + value_tuple = tuple(value_list) # call the actual process - self._previous_parser_process(value, state) + self._previous_parser_process(value_tuple, state) retval = super(MultiOption, self).add_to_parser(parser, ctx) for name in self.opts: our_parser = parser._long_opt.get(name) or parser._short_opt.get(name) if our_parser: - self._eat_all_parser = our_parser + self._eat_all_parser = our_parser # type: ignore[assignment] self._previous_parser_process = our_parser.process - our_parser.process = parser_process + # mypy doesnt like assingment to a method see https://github.com/python/mypy/issues/708 + our_parser.process = parser_process # type: ignore[assignment] break return retval diff --git a/tests/functional/cli/test_multioption.py b/tests/functional/cli/test_multioption.py new file mode 100644 index 00000000000..e9013fdb658 --- /dev/null +++ b/tests/functional/cli/test_multioption.py @@ -0,0 +1,142 @@ +import pytest +from dbt.tests.util import run_dbt + + +model_one_sql = """ +select 1 as fun +""" + +schema_sql = """ +sources: + - name: my_source + description: "My source" + schema: test_schema + tables: + - name: my_table + - name: my_other_table + +exposures: + - name: weekly_jaffle_metrics + label: By the Week + type: dashboard + maturity: high + url: https://bi.tool/dashboards/1 + description: > + Did someone say "exponential growth"? + depends_on: + - ref('model_one') + owner: + name: dbt Labs + email: data@jaffleshop.com +""" + + +class TestResourceType: + @pytest.fixture(scope="class") + def models(self): + return {"schema.yml": schema_sql, "model_one.sql": model_one_sql} + + def test_resource_type_single(self, project): + result = run_dbt(["-q", "ls", "--resource-types", "model"]) + assert len(result) == 1 + assert result == ["test.model_one"] + + def test_resource_type_quoted(self, project): + result = run_dbt(["-q", "ls", "--resource-types", "model source"]) + assert len(result) == 3 + expected_result = { + "test.model_one", + "source:test.my_source.my_table", + "source:test.my_source.my_other_table", + } + assert set(result) == expected_result + + def test_resource_type_args(self, project): + result = run_dbt( + [ + "-q", + "ls", + "--resource-type", + "model", + "--resource-type", + "source", + "--resource-type", + "exposure", + ] + ) + assert len(result) == 4 + expected_result = { + "test.model_one", + "source:test.my_source.my_table", + "source:test.my_source.my_other_table", + "exposure:test.weekly_jaffle_metrics", + } + assert set(result) == expected_result + + +class TestOutputKeys: + @pytest.fixture(scope="class") + def models(self): + return {"model_one.sql": model_one_sql} + + def test_output_key_single(self, project): + result = run_dbt(["-q", "ls", "--output", "json", "--output-keys", "name"]) + assert len(result) == 1 + assert result == ['{"name": "model_one"}'] + + def test_output_key_quoted(self, project): + result = run_dbt(["-q", "ls", "--output", "json", "--output-keys", "name resource_type"]) + + assert len(result) == 1 + assert result == ['{"name": "model_one", "resource_type": "model"}'] + + def test_output_key_args(self, project): + result = run_dbt( + [ + "-q", + "ls", + "--output", + "json", + "--output-keys", + "name", + "--output-keys", + "resource_type", + ] + ) + + assert len(result) == 1 + assert result == ['{"name": "model_one", "resource_type": "model"}'] + + +class TestSelectExclude: + @pytest.fixture(scope="class") + def models(self): + return { + "model_one.sql": model_one_sql, + "model_two.sql": model_one_sql, + "model_three.sql": model_one_sql, + } + + def test_select_exclude_single(self, project): + result = run_dbt(["-q", "ls", "--select", "model_one"]) + assert len(result) == 1 + assert result == ["test.model_one"] + result = run_dbt(["-q", "ls", "--exclude", "model_one"]) + assert len(result) == 2 + assert "test.model_one" not in result + + def test_select_exclude_quoted(self, project): + result = run_dbt(["-q", "ls", "--select", "model_one model_two"]) + assert len(result) == 2 + assert "test.model_three" not in result + result = run_dbt(["-q", "ls", "--exclude", "model_one model_two"]) + assert len(result) == 1 + assert result == ["test.model_three"] + + def test_select_exclude_args(self, project): + result = run_dbt(["-q", "ls", "--select", "model_one", "--select", "model_two"]) + assert len(result) == 2 + assert "test.model_three" not in result + result = run_dbt(["-q", "ls", "--exclude", "model_one", "--exclude", "model_two"]) + assert len(result) == 1 + assert result == ["test.model_three"]