Skip to content

Commit

Permalink
updated tests for coverage, fixed the requirement for one of ProjectC…
Browse files Browse the repository at this point in the history
…onfig.dbt_project_path or ProjectConfig.manifest_path as we now support defining dbt_project_path in ExecutionConfig and RenderConfig
  • Loading branch information
MrBones757 committed Nov 2, 2023
1 parent a655952 commit 3398387
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 17 deletions.
8 changes: 5 additions & 3 deletions cosmos/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,13 @@ def __init__(
manifest_path: str | Path | None = None,
project_name: str | None = None,
):
# Since we allow dbt_project_path to be defined in ExecutionConfig and RenderConfig
# dbt_project_path may not always be defined here.
# We do, however, still require that both manifest_path and project_name be defined, or neither be defined.
if not dbt_project_path:
if not manifest_path or not project_name:
if manifest_path and not project_name or project_name and not manifest_path:
raise CosmosValueError(
"ProjectConfig requires dbt_project_path and/or manifest_path to be defined."
" If only manifest_path is defined, project_name must also be defined."
"If ProjectConfig.dbt_project_path is not defined, ProjectConfig.manifest_path and ProjectConfig.project_name must be defined together, or both left undefined."
)
if project_name:
self.project_name = project_name
Expand Down
3 changes: 0 additions & 3 deletions cosmos/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,6 @@ def __init__(
# Since we now support both project_config.dbt_project_path, render_config.project_path and execution_config.project_path
# We need to ensure that only one interface is being used.
if project_config.dbt_project_path and (render_config.project_path or execution_config.project_path):
print(f"RenderConfig: {render_config.project_path}")
print(f"ExecutionConfig: {execution_config.project_path}")
print(f"ProjectConfig: {project_config.dbt_project_path}")
raise CosmosValueError(
"ProjectConfig.dbt_project_path is mutually exclusive with RenderConfig.dbt_project_path and ExecutionConfig.dbt_project_path."
+ "If using RenderConfig.dbt_project_path or ExecutionConfig.dbt_project_path, ProjectConfig.dbt_project_path should be None"
Expand Down
19 changes: 8 additions & 11 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,14 @@ def test_init_with_manifest_path_and_project_path_succeeds():

def test_init_with_no_params():
"""
The constructor now validates that the required base fields are present
As such, we should test here that the correct exception is raised if these are not correctly defined
This functionality has been moved from the validate method
With the implementation of dbt_project_path in RenderConfig and ExecutionConfig
dbt_project_path becomes optional here. The only requirement is that if one of
manifest_path or project_name is defined, they should both be defined.
We used to enforce dbt_project_path or manifest_path and project_name, but this is
No longer the case
"""
with pytest.raises(CosmosValueError) as err_info:
ProjectConfig()
assert err_info.value.args[0] == (
"ProjectConfig requires dbt_project_path and/or manifest_path to be defined. "
"If only manifest_path is defined, project_name must also be defined."
)
project_config = ProjectConfig()
assert project_config


def test_init_with_manifest_path_and_not_project_path_and_not_project_name_fails():
Expand All @@ -55,8 +53,7 @@ def test_init_with_manifest_path_and_not_project_path_and_not_project_name_fails
with pytest.raises(CosmosValueError) as err_info:
ProjectConfig(manifest_path=DBT_PROJECTS_ROOT_DIR / "manifest.json")
assert err_info.value.args[0] == (
"ProjectConfig requires dbt_project_path and/or manifest_path to be defined. "
"If only manifest_path is defined, project_name must also be defined."
"If ProjectConfig.dbt_project_path is not defined, ProjectConfig.manifest_path and ProjectConfig.project_name must be defined together, or both left undefined."
)


Expand Down
76 changes: 76 additions & 0 deletions tests/test_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,79 @@ def test_converter_fails_execution_config_no_project_dir(mock_load_dbt_graph, ex
err_info.value.args[0]
== "ExecutionConfig.dbt_project_path is required for the execution of dbt tasks in all execution modes."
)


@pytest.mark.parametrize(
"execution_mode,operator_args",
[
(ExecutionMode.KUBERNETES, {}),
# (ExecutionMode.DOCKER, {"image": "sample-image"}),
],
)
@patch("cosmos.converter.DbtGraph.filtered_nodes", nodes)
@patch("cosmos.converter.DbtGraph.load")
def test_converter_fails_project_config_path_and_execution_config_path(
mock_load_dbt_graph, execution_mode, operator_args
):
"""
This test ensures that we fail if we defined project path in ProjectConfig and ExecutionConfig
They are mutually exclusive, so this should be allowed.
"""
project_config = ProjectConfig(dbt_project_path=SAMPLE_DBT_PROJECT.as_posix())
execution_config = ExecutionConfig(execution_mode=execution_mode, dbt_project_path=SAMPLE_DBT_PROJECT.as_posix())
render_config = RenderConfig(emit_datasets=True)
profile_config = ProfileConfig(
profile_name="my_profile_name",
target_name="my_target_name",
profiles_yml_filepath=SAMPLE_PROFILE_YML,
)
with pytest.raises(CosmosValueError) as err_info:
DbtToAirflowConverter(
nodes=nodes,
project_config=project_config,
profile_config=profile_config,
execution_config=execution_config,
render_config=render_config,
operator_args=operator_args,
)
assert (
err_info.value.args[0]
== "ProjectConfig.dbt_project_path is mutually exclusive with RenderConfig.dbt_project_path and ExecutionConfig.dbt_project_path.If using RenderConfig.dbt_project_path or ExecutionConfig.dbt_project_path, ProjectConfig.dbt_project_path should be None"
)


@pytest.mark.parametrize(
"execution_mode,operator_args",
[
(ExecutionMode.KUBERNETES, {}),
# (ExecutionMode.DOCKER, {"image": "sample-image"}),
],
)
@patch("cosmos.converter.DbtGraph.filtered_nodes", nodes)
@patch("cosmos.converter.DbtGraph.load")
def test_converter_fails_no_manifest_no_render_config(mock_load_dbt_graph, execution_mode, operator_args):
"""
This test ensures that we fail if we define project path in ProjectConfig and ExecutionConfig
They are mutually exclusive, so this should be allowed.
"""
project_config = ProjectConfig()
execution_config = ExecutionConfig(execution_mode=execution_mode, dbt_project_path=SAMPLE_DBT_PROJECT.as_posix())
render_config = RenderConfig(emit_datasets=True)
profile_config = ProfileConfig(
profile_name="my_profile_name",
target_name="my_target_name",
profiles_yml_filepath=SAMPLE_PROFILE_YML,
)
with pytest.raises(CosmosValueError) as err_info:
DbtToAirflowConverter(
nodes=nodes,
project_config=project_config,
profile_config=profile_config,
execution_config=execution_config,
render_config=render_config,
operator_args=operator_args,
)
assert (
err_info.value.args[0]
== "RenderConfig.dbt_project_path is required for rendering an airflow DAG from a DBT Graph if no manifest is provided."
)

0 comments on commit 3398387

Please sign in to comment.