From a4e1a59b65559b22cd391c00289e3d85ff3f468f Mon Sep 17 00:00:00 2001 From: Nicholas Yager Date: Mon, 15 Jan 2024 12:57:51 -0500 Subject: [PATCH 1/3] feat: Add check for existing groups. Make the user pick one if multiple are available. --- dbt_meshify/dbt_projects.py | 20 +++++++++++++++++++- dbt_meshify/main.py | 10 +++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/dbt_meshify/dbt_projects.py b/dbt_meshify/dbt_projects.py index a0ed37f..ff4c922 100644 --- a/dbt_meshify/dbt_projects.py +++ b/dbt_meshify/dbt_projects.py @@ -3,7 +3,7 @@ import json import os from pathlib import Path -from typing import Any, Dict, MutableMapping, Optional, Set, Union +from typing import Any, Dict, List, MutableMapping, Optional, Set, Union import yaml from dbt.contracts.graph.manifest import Manifest @@ -309,6 +309,7 @@ def __init__( self.path = path self.dbt = dbt resources = self.select_resources(output_key="unique_id") + self._group_definition_files: Optional[List[Path]] = None super().__init__(manifest, project, catalog, name, resources) self.jinja_blocks: Dict[str, JinjaBlock] = self.find_jinja_blocks() @@ -390,6 +391,23 @@ def split( return subproject + @property + def group_definition_files(self) -> List[Path]: + """A list of files that contain group definitions, in order of how many groups are present.""" + + if self._group_definition_files is not None: + return self._group_definition_files + + paths: Dict[Path, int] = {} + for group in self.manifest.groups.values(): + path = self.path / Path(group.original_file_path) + paths[path] = paths.get(path, 0) + 1 + + self._group_definition_files = list( + map(lambda x: x[0], sorted(paths.items(), key=lambda x: x[1])) + ) + return self._group_definition_files + class DbtSubProject(BaseDbtProject, PathedProject): """ diff --git a/dbt_meshify/main.py b/dbt_meshify/main.py index 3742e43..37b6bb6 100644 --- a/dbt_meshify/main.py +++ b/dbt_meshify/main.py @@ -547,7 +547,15 @@ def create_group( project = DbtProject.from_directory(path, read_catalog) if group_yml_path is None: - group_yml_path = (path / Path("models/_groups.yml")).resolve() + existing_paths = project.group_definition_files() + if len(existing_paths) > 1: + raise FatalMeshifyException( + "Unable to pick which group YAML file to use. Please specify one using the --group-yml-path argument." + ) + if len(existing_paths) == 1: + group_yml_path = existing_paths[0] + else: + group_yml_path = (path / Path("models/_groups.yml")).resolve() else: group_yml_path = Path(group_yml_path).resolve() logger.info(f"Creating new model group in file {group_yml_path.name}") From 66658c3fad9c7d93b5405720c92f3d4bfcc27c4a Mon Sep 17 00:00:00 2001 From: Nicholas Yager Date: Mon, 15 Jan 2024 15:04:01 -0500 Subject: [PATCH 2/3] feat: Add tests --- dbt_meshify/main.py | 2 +- tests/integration/test_group_command.py | 85 +++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/dbt_meshify/main.py b/dbt_meshify/main.py index 37b6bb6..45d27a7 100644 --- a/dbt_meshify/main.py +++ b/dbt_meshify/main.py @@ -547,7 +547,7 @@ def create_group( project = DbtProject.from_directory(path, read_catalog) if group_yml_path is None: - existing_paths = project.group_definition_files() + existing_paths = project.group_definition_files if len(existing_paths) > 1: raise FatalMeshifyException( "Unable to pick which group YAML file to use. Please specify one using the --group-yml-path argument." diff --git a/tests/integration/test_group_command.py b/tests/integration/test_group_command.py index 1c8e22b..033b942 100644 --- a/tests/integration/test_group_command.py +++ b/tests/integration/test_group_command.py @@ -99,3 +99,88 @@ def test_read_catalog_group(): assert result.exit_code != 0 assert "dbt docs generate" not in result.stdout teardown_test_project(dest_path_string) + + +def test_group_name_group_file(): + """Verify that providing a group yaml path results in a group yml being created at the appropriate path.""" + setup_test_project(src_path_string, dest_path_string) + runner = CliRunner() + result = runner.invoke( + cli, + [ + "group", + "test_group", + "--owner-name", + "Teenage Mutant Jinja Turtles", + "--select", + "+order", + "--project-path", + dest_path_string, + "--group-yml-path", + str(Path(dest_path_string) / "models" / "test_groups.yml"), + ], + ) + + assert result.exit_code == 0 + assert (Path(dest_path_string) / "models" / "test_groups.yml").exists() + teardown_test_project(dest_path_string) + + +def test_group_raises_exception_ambiguous_group_file(): + """The group command raises an exception if there are multiple group files present and `--group-yml-path` is not specified.""" + setup_test_project(src_path_string, dest_path_string) + + for index in range(2): + group_yaml = Path(dest_path_string) / "models" / f"groups_{index}.yml" + with open(group_yaml, "w") as file: + file.write(f"groups:\n - name: group_{index}\n owner:\n email: foo@example.com") + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "group", + "test_group", + "--owner-name", + "Teenage Mutant Jinja Turtles", + "--select", + "+order", + "--project-path", + dest_path_string, + ], + ) + + assert result.exit_code != 0 + assert "Unable to pick which group YAML" in result.stdout + teardown_test_project(dest_path_string) + + +def test_defining_group_yaml_disambiguates_group_file(): + """The group command will not raise an exception if there are multiple group files present and `--group-yml-path` is specified.""" + setup_test_project(src_path_string, dest_path_string) + + for index in range(2): + group_yaml = Path(dest_path_string) / "models" / f"groups_{index}.yml" + with open(group_yaml, "w") as file: + file.write(f"groups:\n - name: group_{index}\n owner:\n email: foo@example.com") + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "group", + "test_group", + "--owner-name", + "Teenage Mutant Jinja Turtles", + "--select", + "+order", + "--project-path", + dest_path_string, + "--group-yml-path", + str(Path(dest_path_string) / "models" / "test_groups.yml"), + ], + ) + + assert result.exit_code == 0 + assert (Path(dest_path_string) / "models" / "test_groups.yml").exists() + teardown_test_project(dest_path_string) From 03d97925df903d3336da551f368c75e2139fc42a Mon Sep 17 00:00:00 2001 From: Nicholas Yager Date: Wed, 17 Jan 2024 20:28:06 -0500 Subject: [PATCH 3/3] fix: Remove group file sorting since we don't care about it! --- dbt_meshify/dbt_projects.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dbt_meshify/dbt_projects.py b/dbt_meshify/dbt_projects.py index ff4c922..fe69bcf 100644 --- a/dbt_meshify/dbt_projects.py +++ b/dbt_meshify/dbt_projects.py @@ -398,14 +398,11 @@ def group_definition_files(self) -> List[Path]: if self._group_definition_files is not None: return self._group_definition_files - paths: Dict[Path, int] = {} - for group in self.manifest.groups.values(): - path = self.path / Path(group.original_file_path) - paths[path] = paths.get(path, 0) + 1 + paths: Set[Path] = { + self.path / Path(group.original_file_path) for group in self.manifest.groups.values() + } + self._group_definition_files = list(paths) - self._group_definition_files = list( - map(lambda x: x[0], sorted(paths.items(), key=lambda x: x[1])) - ) return self._group_definition_files