diff --git a/CHANGELOG.md b/CHANGELOG.md index c92f4b54834..0a2f9bd9d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - 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) - Store resolved node names in manifest ([#2647](https://github.com/fishtown-analytics/dbt/issues/2647), [#2837](https://github.com/fishtown-analytics/dbt/pull/2837)) +- 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..6f8bb26d8ce 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -431,11 +431,15 @@ def _update_into(dest: MutableMapping[str, T], new_item: T): class Manifest: """The manifest for the full graph, after parsing and during compilation. """ + # These attributes are both positional and by keyword. If an attribute + # is added it must all be added in the __reduce_ex__ method in the + # args tuple in the right position. nodes: MutableMapping[str, ManifestNode] sources: MutableMapping[str, ParsedSourceDefinition] macros: MutableMapping[str, ParsedMacro] docs: MutableMapping[str, ParsedDocumentation] exposures: MutableMapping[str, ParsedExposure] + selectors: MutableMapping[str, Any] disabled: List[CompileResultNode] files: MutableMapping[str, SourceFile] metadata: ManifestMetadata = field(default_factory=ManifestMetadata) @@ -461,6 +465,7 @@ def from_macros( macros=macros, docs={}, exposures={}, + selectors={}, disabled=[], files=files, ) @@ -730,8 +735,9 @@ def deepcopy(self): macros={k: _deepcopy(v) for k, v in self.macros.items()}, docs={k: _deepcopy(v) for k, v in self.docs.items()}, exposures={k: _deepcopy(v) for k, v in self.exposures.items()}, - disabled=[_deepcopy(n) for n in self.disabled], + selectors=self.root_project.manifest_selectors, metadata=self.metadata, + disabled=[_deepcopy(n) for n in self.disabled], files={k: _deepcopy(v) for k, v in self.files.items()}, ) @@ -749,6 +755,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, @@ -905,7 +912,13 @@ def merge_from_artifact( f'Merged {len(merged)} items from state (sample: {sample})' ) - # provide support for copy.deepcopy() - we jsut need to avoid the lock! + # Provide support for copy.deepcopy() - we just need to avoid the lock! + # pickle and deepcopy use this. It returns a callable object used to + # create the initial version of the object and a tuple of arguments + # for the object, i.e. the Manifest. + # The order of the arguments must match the order of the attributes + # in the Manifest class declaration, because they are used as + # positional arguments to construct a Manifest. def __reduce_ex__(self, protocol): args = ( self.nodes, @@ -913,6 +926,7 @@ def __reduce_ex__(self, protocol): self.macros, self.docs, self.exposures, + self.selectors, self.disabled, self.files, self.metadata, @@ -952,6 +966,11 @@ class WritableManifest(ArtifactMixin): 'The exposures defined in the dbt project and its dependencies' )) ) + selectors: Mapping[UniqueID, Any] = field( + metadata=dict(description=( + 'The selectors defined in selectors.yml' + )) + ) disabled: Optional[List[CompileResultNode]] = field(metadata=dict( description='A list of the disabled nodes in the target' )) 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 9d84d2e0b4e..8e5c9ce9600 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -1610,6 +1610,7 @@ def expected_seeded_manifest(self, model_database=None, quote_model=False): 'maturity': None, } }, + 'selectors': {}, 'parent_map': { 'model.test.model': ['seed.test.seed'], 'model.test.second_model': ['seed.test.seed'], @@ -1988,6 +1989,7 @@ def expected_postgres_references_manifest(self, model_database=None): }, }, 'exposures': {}, + 'selectors': {}, 'docs': { 'dbt.__overview__': ANY, 'test.column_info': { @@ -2542,6 +2544,7 @@ def expected_bigquery_complex_manifest(self): }, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'child_map': { 'model.test.clustered': [], 'model.test.multi_clustered': [], @@ -2811,6 +2814,7 @@ def expected_redshift_incremental_view_manifest(self): }, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': { 'model.test.model': ['seed.test.seed'], 'seed.test.seed': [], @@ -2853,7 +2857,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/rpc/util.py b/test/rpc/util.py index f792cbbf9a5..4c3e615a1f0 100644 --- a/test/rpc/util.py +++ b/test/rpc/util.py @@ -402,11 +402,14 @@ def get_manifest(self, request_id=1): ) def is_result(self, data: Dict[str, Any], id=None) -> Dict[str, Any]: + if id is not None: assert data['id'] == id assert data['jsonrpc'] == '2.0' - assert 'result' in data + if 'error' in data: + print(data['error']['message']) assert 'error' not in data + assert 'result' in data return data['result'] def is_async_result(self, data: Dict[str, Any], id=None) -> str: 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 6c63f122f47..c88c6c2178e 100644 --- a/test/unit/test_manifest.py +++ b/test/unit/test_manifest.py @@ -225,7 +225,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( @@ -235,6 +235,7 @@ def test__no_nodes(self): 'sources': {}, 'macros': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'metadata': { @@ -254,7 +255,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() @@ -321,7 +322,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'] @@ -366,7 +367,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( @@ -376,6 +377,7 @@ def test_no_nodes_with_metadata(self, mock_user): 'sources': {}, 'macros': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'docs': {}, @@ -396,7 +398,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): @@ -422,7 +424,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'), @@ -607,7 +609,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(), @@ -616,6 +618,7 @@ def test__no_nodes(self): 'macros': {}, 'sources': {}, 'exposures': {}, + 'selectors': {}, 'parent_map': {}, 'child_map': {}, 'metadata': { @@ -634,7 +637,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() @@ -699,7 +702,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 @@ -749,6 +752,7 @@ def setUp(self): disabled=[], files={}, exposures={}, + selectors={}, ) @@ -769,6 +773,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):