From 4efab3edcfa6023b17fd8d6263bf11c84d822e2e Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Sat, 7 Nov 2020 16:00:41 -0500 Subject: [PATCH] Save selector dictionary and write out in manifest [#2693][#2800] --- CHANGELOG.md | 1 + core/dbt/config/profile.py | 2 ++ core/dbt/config/project.py | 13 +++++++++++ core/dbt/config/runtime.py | 2 ++ core/dbt/contracts/graph/manifest.py | 10 ++++++++ core/dbt/contracts/selection.py | 1 + core/dbt/parser/manifest.py | 1 + .../test_graph_selection.py | 23 ++++++++++++++++++- .../test_docs_generate.py | 6 ++++- test/unit/test_compiler.py | 5 ++++ test/unit/test_graph_selector_methods.py | 1 + test/unit/test_graph_selector_parsing.py | 2 ++ test/unit/test_manifest.py | 23 +++++++++++-------- test/unit/test_parser.py | 3 ++- 14 files changed, 81 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85ce4798484..9cdaf592c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features - Added macro get_partitions_metadata(table) to return partition metadata for partitioned table [#2596](https://github.com/fishtown-analytics/dbt/pull/2596) - Added native python 're' module for regex in jinja templates [#2851](https://github.com/fishtown-analytics/dbt/pull/2851) +- Save selectors dictionary to manifest, allow descriptions ([#2693](https://github.com/fishtown-analytics/dbt/issues/2693), [#2866](https://github.com/fishtown-analytics/dbt/pull/2866)) ### Fixes - Respect --project-dir in dbt clean command ([#2840](https://github.com/fishtown-analytics/dbt/issues/2840), [#2841](https://github.com/fishtown-analytics/dbt/pull/2841)) diff --git a/core/dbt/config/profile.py b/core/dbt/config/profile.py index 400b567c820..49c576ba218 100644 --- a/core/dbt/config/profile.py +++ b/core/dbt/config/profile.py @@ -81,6 +81,8 @@ def read_user_config(directory: str) -> UserConfig: return UserConfig() +# The Profile class is included in RuntimeConfig, so any attribute +# additions must also be set where the RuntimeConfig class is created @dataclass class Profile(HasCredentials): profile_name: str diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index d8f1fd267f7..f8ea67fda1c 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -369,6 +369,14 @@ def create_project(self, rendered: RenderComponents) -> 'Project': query_comment = _query_comment_from_cfg(cfg.query_comment) packages = package_config_from_data(rendered.packages_dict) + manifest_selectors: Dict[str, Any] = {} + if rendered.selectors_dict: + # this is a dict with a single key 'selectors' pointing to a list + # of dicts. + if rendered.selectors_dict['selectors']: + # for each selector dict, transform into 'name': { } + for sel in rendered.selectors_dict['selectors']: + manifest_selectors[sel['name']] = sel selectors = selector_config_from_data(rendered.selectors_dict) project = Project( @@ -396,6 +404,7 @@ def create_project(self, rendered: RenderComponents) -> 'Project': snapshots=snapshots, dbt_version=dbt_version, packages=packages, + manifest_selectors=manifest_selectors, selectors=selectors, query_comment=query_comment, sources=sources, @@ -458,6 +467,7 @@ def from_project_root( class VarProvider: """Var providers are tied to a particular Project.""" + def __init__( self, vars: Dict[str, Dict[str, Any]] @@ -476,6 +486,8 @@ def to_dict(self): return self.vars +# The Project class is included in RuntimeConfig, so any attribute +# additions must also be set where the RuntimeConfig class is created @dataclass class Project: project_name: str @@ -504,6 +516,7 @@ class Project: vars: VarProvider dbt_version: List[VersionSpecifier] packages: Dict[str, Any] + manifest_selectors: Dict[str, Any] selectors: SelectorConfig query_comment: QueryComment config_version: int diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index f3dbb4a36bb..f433ac411fd 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -106,6 +106,7 @@ def from_parts( snapshots=project.snapshots, dbt_version=project.dbt_version, packages=project.packages, + manifest_selectors=project.manifest_selectors, selectors=project.selectors, query_comment=project.query_comment, sources=project.sources, @@ -483,6 +484,7 @@ def from_parts( snapshots=project.snapshots, dbt_version=project.dbt_version, packages=project.packages, + manifest_selectors=project.manifest_selectors, selectors=project.selectors, query_comment=project.query_comment, sources=project.sources, diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index e1daa4193b5..f001f633bc2 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -438,6 +438,7 @@ class Manifest: exposures: MutableMapping[str, ParsedExposure] disabled: List[CompileResultNode] files: MutableMapping[str, SourceFile] + selectors: MutableMapping[str, Any] metadata: ManifestMetadata = field(default_factory=ManifestMetadata) flat_graph: Dict[str, Any] = field(default_factory=dict) _docs_cache: Optional[DocCache] = None @@ -462,6 +463,7 @@ def from_macros( docs={}, exposures={}, disabled=[], + selectors={}, files=files, ) @@ -732,6 +734,7 @@ def deepcopy(self): exposures={k: _deepcopy(v) for k, v in self.exposures.items()}, disabled=[_deepcopy(n) for n in self.disabled], metadata=self.metadata, + selectors=self.root_project.manifest_selectors, files={k: _deepcopy(v) for k, v in self.files.items()}, ) @@ -749,6 +752,7 @@ def writable_manifest(self): macros=self.macros, docs=self.docs, exposures=self.exposures, + selectors=self.selectors, metadata=self.metadata, disabled=self.disabled, child_map=forward_edges, @@ -913,6 +917,7 @@ def __reduce_ex__(self, protocol): self.macros, self.docs, self.exposures, + self.selectors, self.disabled, self.files, self.metadata, @@ -942,6 +947,11 @@ class WritableManifest(ArtifactMixin): 'The macros defined in the dbt project and its dependencies' )) ) + selectors: Mapping[UniqueID, Any] = field( + metadata=dict(description=( + 'The selectors defined in selectors.yml' + )) + ) docs: Mapping[UniqueID, ParsedDocumentation] = field( metadata=dict(description=( 'The docs defined in the dbt project and its dependencies' diff --git a/core/dbt/contracts/selection.py b/core/dbt/contracts/selection.py index d193473ecb8..a5bba15c749 100644 --- a/core/dbt/contracts/selection.py +++ b/core/dbt/contracts/selection.py @@ -8,6 +8,7 @@ class SelectorDefinition(JsonSchemaMixin): name: str definition: Union[str, Dict[str, Any]] + description: str = '' @dataclass diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index feafc3a21fb..3efef57874b 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -318,6 +318,7 @@ def create_manifest(self) -> Manifest: metadata=self.root_project.get_metadata(), disabled=disabled, files=self.results.files, + selectors=self.root_project.manifest_selectors, ) manifest.patch_nodes(self.results.patches) manifest.patch_macros(self.results.macro_patches) diff --git a/test/integration/007_graph_selection_tests/test_graph_selection.py b/test/integration/007_graph_selection_tests/test_graph_selection.py index ce4b79a6fe5..88f291baa29 100644 --- a/test/integration/007_graph_selection_tests/test_graph_selection.py +++ b/test/integration/007_graph_selection_tests/test_graph_selection.py @@ -1,4 +1,7 @@ from test.integration.base import DBTIntegrationTest, use_profile +import yaml +import json +import os class TestGraphSelection(DBTIntegrationTest): @@ -10,6 +13,18 @@ def schema(self): def models(self): return "models" + @property + def selectors_config(self): + return yaml.safe_load(''' + selectors: + - name: bi_selector + description: This is a BI selector + definition: + method: tag + value: bi + ''') + + def assert_correct_schemas(self): with self.get_connection(): exists = self.adapter.check_schema_exists( @@ -43,7 +58,7 @@ def test__postgres__specific_model(self): def test__postgres__tags(self): self.run_sql_file("seed.sql") - results = self.run_dbt(['run', '--models', 'tag:bi']) + results = self.run_dbt(['run', '--selector', 'bi_selector']) self.assertEqual(len(results), 2) created_models = self.get_models_in_schema() @@ -52,6 +67,12 @@ def test__postgres__tags(self): self.assertTrue('users' in created_models) self.assertTrue('users_rollup' in created_models) self.assert_correct_schemas() + self.assertTrue(os.path.exists('./target/manifest.json')) + with open('./target/manifest.json') as fp: + manifest = json.load(fp) + self.assertTrue('selectors' in manifest) + + @use_profile('postgres') def test__postgres__tags_and_children(self): diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py index 109e8cd6f06..7c481af8b5c 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -1503,6 +1503,7 @@ def expected_seeded_manifest(self, model_database=None): 'maturity': None, } }, + 'selectors': {}, 'parent_map': { 'model.test.model': ['seed.test.seed'], 'model.test.second_model': ['seed.test.seed'], @@ -1825,6 +1826,7 @@ def expected_postgres_references_manifest(self, model_database=None): }, }, 'exposures': {}, + 'selectors': {}, 'docs': { 'dbt.__overview__': ANY, 'test.column_info': { @@ -2322,6 +2324,7 @@ def expected_bigquery_complex_manifest(self): }, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'child_map': { 'model.test.clustered': [], 'model.test.multi_clustered': [], @@ -2533,6 +2536,7 @@ def expected_redshift_incremental_view_manifest(self): }, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': { 'model.test.model': ['seed.test.seed'], 'seed.test.seed': [] @@ -2572,7 +2576,7 @@ def verify_manifest(self, expected_manifest): manifest_keys = frozenset({ 'nodes', 'sources', 'macros', 'parent_map', 'child_map', - 'docs', 'metadata', 'docs', 'disabled', 'exposures' + 'docs', 'metadata', 'docs', 'disabled', 'exposures', 'selectors', }) self.assertEqual(frozenset(manifest), manifest_keys) diff --git a/test/unit/test_compiler.py b/test/unit/test_compiler.py index 7ed6ec11f11..775b6af0c3c 100644 --- a/test/unit/test_compiler.py +++ b/test/unit/test_compiler.py @@ -154,6 +154,7 @@ def test__prepend_ctes__already_has_cte(self): disabled=[], files={}, exposures={}, + selectors={}, ) compiler = dbt.compilation.Compiler(self.config) @@ -236,6 +237,7 @@ def test__prepend_ctes__no_ctes(self): disabled=[], files={}, exposures={}, + selectors={}, ) compiler = dbt.compilation.Compiler(self.config) @@ -327,6 +329,7 @@ def test__prepend_ctes(self): disabled=[], files={}, exposures={}, + selectors={}, ) compiler = dbt.compilation.Compiler(self.config) @@ -430,6 +433,7 @@ def test__prepend_ctes__cte_not_compiled(self): disabled=[], files={}, exposures={}, + selectors={}, ) compiler = dbt.compilation.Compiler(self.config) @@ -534,6 +538,7 @@ def test__prepend_ctes__multiple_levels(self): disabled=[], files={}, exposures={}, + selectors={}, ) compiler = dbt.compilation.Compiler(self.config) diff --git a/test/unit/test_graph_selector_methods.py b/test/unit/test_graph_selector_methods.py index a3801c096cc..69d1fe2bf54 100644 --- a/test/unit/test_graph_selector_methods.py +++ b/test/unit/test_graph_selector_methods.py @@ -471,6 +471,7 @@ def manifest(seed, source, ephemeral_model, view_model, table_model, ext_source, files={}, exposures={}, disabled=[], + selectors={}, ) return manifest diff --git a/test/unit/test_graph_selector_parsing.py b/test/unit/test_graph_selector_parsing.py index 7004694afca..759930dd9dc 100644 --- a/test/unit/test_graph_selector_parsing.py +++ b/test/unit/test_graph_selector_parsing.py @@ -96,11 +96,13 @@ def test_parse_simple(): sf = parse_file('''\ selectors: - name: tagged_foo + description: Selector for foo-tagged models definition: tag: foo ''') assert len(sf.selectors) == 1 + assert sf.selectors[0].description == 'Selector for foo-tagged models' parsed = cli.parse_from_selectors_definition(sf) assert len(parsed) == 1 assert 'tagged_foo' in parsed diff --git a/test/unit/test_manifest.py b/test/unit/test_manifest.py index c1187499b74..0122557d0f6 100644 --- a/test/unit/test_manifest.py +++ b/test/unit/test_manifest.py @@ -224,7 +224,7 @@ def tearDown(self): def test__no_nodes(self): manifest = Manifest( nodes={}, sources={}, macros={}, docs={}, disabled=[], files={}, - exposures={}, + exposures={}, selectors={}, metadata=ManifestMetadata(generated_at=datetime.utcnow()), ) self.assertEqual( @@ -234,6 +234,7 @@ def test__no_nodes(self): 'sources': {}, 'macros': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'metadata': { @@ -253,7 +254,7 @@ def test__nested_nodes(self): nodes = copy.copy(self.nested_nodes) manifest = Manifest( nodes=nodes, sources={}, macros={}, docs={}, disabled=[], files={}, - exposures={}, + exposures={}, selectors={}, metadata=ManifestMetadata(generated_at=datetime.utcnow()), ) serialized = manifest.writable_manifest().to_dict() @@ -320,7 +321,7 @@ def test__build_flat_graph(self): nodes = copy.copy(self.nested_nodes) sources = copy.copy(self.sources) manifest = Manifest(nodes=nodes, sources=sources, macros={}, docs={}, - disabled=[], files={}, exposures={}) + disabled=[], files={}, exposures={}, selectors={}) manifest.build_flat_graph() flat_graph = manifest.flat_graph flat_nodes = flat_graph['nodes'] @@ -365,7 +366,7 @@ def test_no_nodes_with_metadata(self, mock_user): generated_at=datetime.utcnow(), ) manifest = Manifest(nodes={}, sources={}, macros={}, docs={}, - disabled=[], + disabled=[], selectors={}, metadata=metadata, files={}, exposures={}) self.assertEqual( @@ -375,6 +376,7 @@ def test_no_nodes_with_metadata(self, mock_user): 'sources': {}, 'macros': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'docs': {}, @@ -395,7 +397,7 @@ def test_no_nodes_with_metadata(self, mock_user): def test_get_resource_fqns_empty(self): manifest = Manifest(nodes={}, sources={}, macros={}, docs={}, - disabled=[], files={}, exposures={}) + disabled=[], files={}, exposures={}, selectors={}) self.assertEqual(manifest.get_resource_fqns(), {}) def test_get_resource_fqns(self): @@ -421,7 +423,7 @@ def test_get_resource_fqns(self): checksum=FileHash.empty(), ) manifest = Manifest(nodes=nodes, sources=self.sources, macros={}, docs={}, - disabled=[], files={}, exposures={}) + disabled=[], files={}, exposures={}, selectors={}) expect = { 'models': frozenset([ ('snowplow', 'events'), @@ -604,7 +606,7 @@ def tearDown(self): @freezegun.freeze_time('2018-02-14T09:15:13Z') def test__no_nodes(self): metadata = ManifestMetadata(generated_at=datetime.utcnow(), invocation_id='01234567-0123-0123-0123-0123456789ab') - manifest = Manifest(nodes={}, sources={}, macros={}, docs={}, + manifest = Manifest(nodes={}, sources={}, macros={}, docs={}, selectors={}, disabled=[], metadata=metadata, files={}, exposures={}) self.assertEqual( manifest.writable_manifest().to_dict(), @@ -613,6 +615,7 @@ def test__no_nodes(self): 'macros': {}, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'metadata': { @@ -631,7 +634,7 @@ def test__no_nodes(self): def test__nested_nodes(self): nodes = copy.copy(self.nested_nodes) manifest = Manifest(nodes=nodes, sources={}, macros={}, docs={}, - disabled=[], + disabled=[], selectors={}, metadata=ManifestMetadata(generated_at=datetime.utcnow()), files={}, exposures={}) serialized = manifest.writable_manifest().to_dict() @@ -696,7 +699,7 @@ def test__nested_nodes(self): def test__build_flat_graph(self): nodes = copy.copy(self.nested_nodes) manifest = Manifest(nodes=nodes, sources={}, macros={}, docs={}, - disabled=[], + disabled=[], selectors={}, files={}, exposures={}) manifest.build_flat_graph() flat_graph = manifest.flat_graph @@ -746,6 +749,7 @@ def setUp(self): disabled=[], files={}, exposures={}, + selectors={}, ) @@ -766,6 +770,7 @@ def make_manifest(nodes=[], sources=[], macros=[], docs=[]): disabled=[], files={}, exposures={}, + selectors={}, ) diff --git a/test/unit/test_parser.py b/test/unit/test_parser.py index 9dc2242cee0..9064a2e3af2 100644 --- a/test/unit/test_parser.py +++ b/test/unit/test_parser.py @@ -860,7 +860,8 @@ def setUp(self): self.doc.unique_id: self.doc, } self.manifest = Manifest( - nodes=nodes, sources=sources, macros={}, docs=docs, disabled=[], files={}, exposures={} + nodes=nodes, sources=sources, macros={}, docs=docs, + disabled=[], files={}, exposures={}, selectors={}, ) def test_process_docs(self):