Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use MethodName.File when value ends with .py #5295

Merged
merged 8 commits into from
Jul 29, 2022
Merged

use MethodName.File when value ends with .py #5295

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b430320
use MethodName.File when value ends with .py
May 24, 2022
15f1d02
changie new
May 26, 2022
f41cb30
add tests for table_model_py
May 27, 2022
b53df10
remove support for ephemeral models because they're not in the python…
May 27, 2022
da00f58
fix format
ChenyuLInx Jul 28, 2022
9e200ed
Update test_generic_tests.py
ChenyuLInx Jul 28, 2022
75bc980
Update test_base.py
ChenyuLInx Jul 28, 2022
de92a2b
Update files.py
ChenyuLInx Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/unreleased/Features-20220526-165323.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Features
body: allows user to include the file extension for .py models in the dbt run -m command.
time: 2022-05-26T16:53:23.389671-07:00
custom:
Author: leoebfolsom
Issue: "5289"
PR: "5295"
2 changes: 1 addition & 1 deletion core/dbt/graph/selector_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def __post_init__(self):
def default_method(cls, value: str) -> MethodName:
if _probably_path(value):
return MethodName.Path
elif value.lower().endswith(".sql"):
elif value.lower().endswith((".sql", ".py")):
return MethodName.File
else:
return MethodName.FQN
Expand Down
29 changes: 22 additions & 7 deletions test/unit/test_graph_selector_methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,19 @@ def table_model(ephemeral_model):
)


@pytest.fixture
def table_model_py(seed):
return make_model(
'pkg',
'table_model_py',
'select * from {{ ref("seed") }}',
config_kwargs={'materialized': 'table'},
refs=[seed],
tags=[],
path='subdirectory/table_model.py'
)


@pytest.fixture
def ext_source():
return make_source(
Expand Down Expand Up @@ -533,7 +546,6 @@ def union_model(seed, ext_source):
tags=['unions'],
)


@pytest.fixture
def table_id_unique(table_model):
return make_unique_test('pkg', table_model, 'id')
Expand Down Expand Up @@ -590,13 +602,12 @@ def namespaced_union_model(seed, ext_source):
tags=['unions'],
)


@pytest.fixture
def manifest(seed, source, ephemeral_model, view_model, table_model, ext_source, ext_model, union_model, ext_source_2,
def manifest(seed, source, ephemeral_model, view_model, table_model, table_model_py, ext_source, ext_model, union_model, ext_source_2,
ext_source_other, ext_source_other_2, table_id_unique, table_id_not_null, view_id_unique, ext_source_id_unique,
view_test_nothing, namespaced_seed, namespace_model, namespaced_union_model, macro_test_unique, macro_default_test_unique,
macro_test_not_null, macro_default_test_not_null):
nodes = [seed, ephemeral_model, view_model, table_model, union_model, ext_model,
nodes = [seed, ephemeral_model, view_model, table_model, table_model_py, union_model, ext_model,
table_id_unique, table_id_not_null, view_id_unique, ext_source_id_unique, view_test_nothing,
namespaced_seed, namespace_model, namespaced_union_model]
sources = [source, ext_source, ext_source_2,
Expand Down Expand Up @@ -634,7 +645,7 @@ def test_select_fqn(manifest):
assert not search_manifest_using_method(manifest, method, 'ext.unions')
# sources don't show up, because selection pretends they have no FQN. Should it?
assert search_manifest_using_method(manifest, method, 'pkg') == {
'union_model', 'table_model', 'view_model', 'ephemeral_model', 'seed',
'union_model', 'table_model', 'table_model_py', 'view_model', 'ephemeral_model', 'seed',
'mynamespace.union_model', 'mynamespace.ephemeral_model', 'mynamespace.seed'}
assert search_manifest_using_method(
manifest, method, 'ext') == {'ext_model'}
Expand Down Expand Up @@ -713,10 +724,14 @@ def test_select_file(manifest):

assert search_manifest_using_method(
manifest, method, 'table_model.sql') == {'table_model'}
assert search_manifest_using_method(
manifest, method, 'table_model.py') == {'table_model_py'}
assert search_manifest_using_method(
manifest, method, 'union_model.sql') == {'union_model', 'mynamespace.union_model'}
assert not search_manifest_using_method(
manifest, method, 'missing.sql')
assert not search_manifest_using_method(
manifest, method, 'missing.py')


def test_select_package(manifest):
Expand All @@ -725,7 +740,7 @@ def test_select_package(manifest):
assert isinstance(method, PackageSelectorMethod)
assert method.arguments == []

assert search_manifest_using_method(manifest, method, 'pkg') == {'union_model', 'table_model', 'view_model', 'ephemeral_model',
assert search_manifest_using_method(manifest, method, 'pkg') == {'union_model', 'table_model', 'table_model_py', 'view_model', 'ephemeral_model',
'seed', 'raw.seed', 'unique_table_model_id', 'not_null_table_model_id', 'unique_view_model_id', 'view_test_nothing',
'mynamespace.seed', 'mynamespace.ephemeral_model', 'mynamespace.union_model',
}
Expand All @@ -744,7 +759,7 @@ def test_select_config_materialized(manifest):
assert search_manifest_using_method(manifest, method, 'view') == {
'view_model', 'ext_model'}
assert search_manifest_using_method(manifest, method, 'table') == {
'table_model', 'union_model', 'mynamespace.union_model'}
'table_model', 'table_model_py', 'union_model', 'mynamespace.union_model'}


def test_select_test_name(manifest):
Expand Down
1 change: 1 addition & 0 deletions tests/adapter/dbt/tests/adapter/basic/files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@

base_materialized_var_sql = config_materialized_var + model_base
base_table_sql = config_materialized_table + model_base
base_table_py = config_materialized_table + model_base
base_view_sql = config_materialized_view + model_base
base_ephemeral_sql = config_materialized_ephemeral + model_base
ephemeral_with_cte_sql = config_materialized_ephemeral + model_ephemeral_with_cte
Expand Down
2 changes: 2 additions & 0 deletions tests/adapter/dbt/tests/adapter/basic/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
seeds_base_csv,
base_view_sql,
base_table_sql,
base_table_py,
base_materialized_var_sql,
schema_base_yml,
)
Expand All @@ -21,6 +22,7 @@ def models(self):
return {
"view_model.sql": base_view_sql,
"table_model.sql": base_table_sql,
"table_model.py": base_table_py,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably don't want to add this to the base adapter tests as not all adapters support running this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the unittest above looks great to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, and it looks like you made that change, so you don't need anything from me?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw your other comment--looks good to me! Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just went ahead and did it thinking that it would be great for this to be included in the Beta release coming up soon!

Thank you so much for contributing this and the super quick response!!!

"swappable.sql": base_materialized_var_sql,
"schema.yml": schema_base_yml,
}
Expand Down
2 changes: 2 additions & 0 deletions tests/adapter/dbt/tests/adapter/basic/test_generic_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
generic_test_seed_yml,
base_view_sql,
base_table_sql,
base_table_py,
schema_base_yml,
generic_test_view_yml,
generic_test_table_yml,
Expand All @@ -28,6 +29,7 @@ def models(self):
return {
"view_model.sql": base_view_sql,
"table_model.sql": base_table_sql,
"table_model.py": base_table_py,
"schema.yml": schema_base_yml,
"schema_view.yml": generic_test_view_yml,
"schema_table.yml": generic_test_table_yml,
Expand Down