PR feedback: make report selector more like source selector, remove r…

…eports from fqn slector

Make some corresponding fqn adjustments
Add dbt ls report output
Fix dbt ls source output
The default selector now also returns reports
Update tests

core/dbt/contracts/graph/parsed.py

@@ -652,6 +652,10 @@ class ParsedReport(UnparsedBaseNode, HasUniqueID, HasFqn):
     def depends_on_nodes(self):
         return self.depends_on.nodes
 
+    @property
+    def search_name(self):
+        return self.name
+
     # no tags for now, but we could definitely add them
     @property
     def tags(self):

core/dbt/graph/cli.py

@@ -18,7 +18,7 @@
 
 INTERSECTION_DELIMITER = ','
 
-DEFAULT_INCLUDES: List[str] = ['fqn:*', 'source:*']
+DEFAULT_INCLUDES: List[str] = ['fqn:*', 'source:*', 'report:*']
 DEFAULT_EXCLUDES: List[str] = []
 DATA_TEST_SELECTOR: str = 'test_type:data'
 SCHEMA_TEST_SELECTOR: str = 'test_type:schema'

core/dbt/graph/selector_methods.py

@@ -46,6 +46,7 @@ class MethodName(StrEnum):
     TestType = 'test_type'
     ResourceType = 'resource_type'
     State = 'state'
+    Report = 'report'
 
 
 def is_selected_node(real_node, node_selector):
@@ -189,7 +190,7 @@ def search(
         :param str selector: The selector or node name
         """
         qualified_name = selector.split(".")
-        parsed_nodes = list(self.non_source_nodes(included_nodes))
+        parsed_nodes = list(self.parsed_nodes(included_nodes))
         package_names = {n.package_name for _, n in parsed_nodes}
         for node, real_node in parsed_nodes:
             if self.node_is_match(
@@ -237,8 +238,37 @@ def search(
                 continue
             if target_source not in (real_node.source_name, SELECTOR_GLOB):
                 continue
-            if target_table in (None, real_node.name, SELECTOR_GLOB):
-                yield node
+            if target_table not in (None, real_node.name, SELECTOR_GLOB):
+                continue
+
+            yield node
+
+
+class ReportSelectorMethod(SelectorMethod):
+    def search(
+        self, included_nodes: Set[UniqueId], selector: str
+    ) -> Iterator[UniqueId]:
+        parts = selector.split('.')
+        target_package = SELECTOR_GLOB
+        if len(parts) == 1:
+            target_name = parts[0]
+        elif len(parts) == 2:
+            target_package, target_name = parts
+        else:
+            msg = (
+                'Invalid report selector value "{}". Reports must be of '
+                'the form ${{report_name}} or '
+                '${{report_package.report_name}}'
+            ).format(selector)
+            raise RuntimeException(msg)
+
+        for node, real_node in self.report_nodes(included_nodes):
+            if target_package not in (real_node.package_name, SELECTOR_GLOB):
+                continue
+            if target_name not in (real_node.name, SELECTOR_GLOB):
+                continue
+
+            yield node
 
 
 class PathSelectorMethod(SelectorMethod):
@@ -469,6 +499,7 @@ class MethodManager:
         MethodName.TestName: TestNameSelectorMethod,
         MethodName.TestType: TestTypeSelectorMethod,
         MethodName.State: StateSelectorMethod,
+        MethodName.Report: ReportSelectorMethod,
     }
 
     def __init__(

core/dbt/parser/schemas.py

@@ -792,13 +792,15 @@ def __init__(self, schema_parser: SchemaParser, yaml: YamlBlock):
     def parse_report(self, unparsed: UnparsedReport) -> ParsedReport:
         package_name = self.project.project_name
         unique_id = f'{NodeType.Report}.{package_name}.{unparsed.name}'
-        fqn_path = os.path.join('reports', self.yaml.path.relative_path)
-        fqn = self.schema_parser.get_fqn_prefix(fqn_path)
+        path = self.yaml.path.relative_path
+
+        fqn = self.schema_parser.get_fqn_prefix(path)
         fqn.append(unparsed.name)
+
         parsed = ParsedReport(
             package_name=package_name,
             root_path=self.project.project_root,
-            path=self.yaml.file.path.relative_path,
+            path=path,
             original_file_path=self.yaml.path.original_file_path,
             unique_id=unique_id,
             fqn=fqn,

core/dbt/task/list.py

@@ -1,6 +1,10 @@
 import json
 from typing import Type
 
+from dbt.contracts.graph.parsed import (
+    ParsedReport,
+    ParsedSourceDefinition,
+)
 from dbt.graph import (
     parse_difference,
     ResourceTypeSelector,
@@ -82,18 +86,25 @@ def _iterate_selected_nodes(self):
 
     def generate_selectors(self):
         for node in self._iterate_selected_nodes():
-            selector = '.'.join(node.fqn)
             if node.resource_type == NodeType.Source:
-                yield 'source:{}'.format(selector)
+                assert isinstance(node, ParsedSourceDefinition)
+                # sources are searched for by pkg.source_name.table_name
+                source_selector = '.'.join([
+                    node.package_name, node.source_name, node.name
+                ])
+                yield f'source:{source_selector}'
+            elif node.resource_type == NodeType.Report:
+                assert isinstance(node, ParsedReport)
+                # reports are searched for by pkg.report_name
+                report_selector = '.'.join([node.package_name, node.name])
+                yield f'report:{report_selector}'
             else:
-                yield selector
+                # everything else is from `fqn`
+                yield '.'.join(node.fqn)
 
     def generate_names(self):
         for node in self._iterate_selected_nodes():
-            if node.resource_type == NodeType.Source:
-                yield '{0.source_name}.{0.name}'.format(node)
-            else:
-                yield node.name
+            yield node.search_name
 
     def generate_json(self):
         for node in self._iterate_selected_nodes():

test/integration/007_graph_selection_tests/test_graph_selection.py

@@ -358,15 +358,15 @@ def test__postgres__concat_exclude_concat(self):
     @use_profile('postgres')
     def test__postgres__report_parents(self):
         self.run_sql_file("seed.sql")
-        results = self.run_dbt(['ls', '--select', '+test.reports.seed_ml_report'])
+        results = self.run_dbt(['ls', '--select', '+report:seed_ml_report'])
         assert len(results) == 2
-        assert sorted(results) == ['source:test.raw.seed', 'test.reports.seed_ml_report']
+        assert sorted(results) == ['report:test.seed_ml_report', 'source:test.raw.seed']
 
-        results = self.run_dbt(['ls', '--select', '1+test.reports.user_report'])
+        results = self.run_dbt(['ls', '--select', '1+report:user_report'])
         assert len(results) == 3
-        assert sorted(results) == ['test.reports.user_report', 'test.users', 'test.users_rollup']
+        assert sorted(results) == ['report:test.user_report', 'test.users', 'test.users_rollup']
 
-        self.run_dbt(['run', '-m', '+test.reports.user_report'])
+        self.run_dbt(['run', '-m', '+report:user_report'])
         # users, users_rollup
         assert len(results) == 3
 

test/integration/029_docs_generate_tests/test_docs_generate.py

@@ -1444,7 +1444,7 @@ def expected_seeded_manifest(self, model_database=None):
                         'nodes': ['model.test.model', 'model.test.second_model']
                     },
                     'description': 'A description of the complex report',
-                    'fqn': ['test', 'reports', 'notebook_report'],
+                    'fqn': ['test', 'notebook_report'],
                     'maturity': 'medium',
                     'name': 'notebook_report',
                     'original_file_path': self.dir('models/schema.yml'),
@@ -1471,7 +1471,7 @@ def expected_seeded_manifest(self, model_database=None):
                         ],
                     },
                     'description': None,
-                    'fqn': ['test', 'reports', 'simple_report'],
+                    'fqn': ['test', 'simple_report'],
                     'name': 'simple_report',
                     'original_file_path': self.dir('models/schema.yml'),
                     'owner': {

test/integration/062_defer_state_test/test_modified_state.py

@@ -72,7 +72,7 @@ def test_postgres_changed_seed_contents_state(self):
 
         results = self.run_dbt(['ls', '--select', 'state:modified+', '--state', './state'])
         assert len(results) == 7
-        assert set(results) == {'test.seed', 'test.table_model', 'test.view_model', 'test.ephemeral_model', 'test.schema_test.not_null_view_model_id', 'test.schema_test.unique_view_model_id', 'test.reports.my_report'}
+        assert set(results) == {'test.seed', 'test.table_model', 'test.view_model', 'test.ephemeral_model', 'test.schema_test.not_null_view_model_id', 'test.schema_test.unique_view_model_id', 'report:test.my_report'}
 
         shutil.rmtree('./state')
         self.copy_state()

test/unit/test_graph_selector_methods.py

@@ -10,6 +10,7 @@
     DependsOn,
     NodeConfig,
     ParsedModelNode,
+    ParsedReport,
     ParsedSeedNode,
     ParsedSnapshotNode,
     ParsedDataTestNode,
@@ -20,6 +21,7 @@
     ColumnInfo,
 )
 from dbt.contracts.graph.manifest import Manifest
+from dbt.contracts.graph.unparsed import ExposureType, ReportOwner
 from dbt.contracts.state import PreviousState
 from dbt.node_types import NodeType
 from dbt.graph.selector_methods import (
@@ -33,6 +35,7 @@
     TestNameSelectorMethod,
     TestTypeSelectorMethod,
     StateSelectorMethod,
+    ReportSelectorMethod,
 )
 import dbt.exceptions
 import dbt.contracts.graph.parsed
@@ -291,6 +294,30 @@ def make_data_test(pkg, name, sql, refs=None, sources=None, tags=None, path=None
     )
 
 
+def make_report(pkg, name, path=None, fqn_extras=None, owner=None):
+    if path is None:
+        path = 'schema.yml'
+
+    if fqn_extras is None:
+        fqn_extras = []
+
+    if owner is None:
+        owner = ReportOwner(email='test@example.com')
+
+    fqn = [pkg, 'reports'] + fqn_extras + [name]
+    return ParsedReport(
+        name=name,
+        type=ExposureType.Notebook,
+        fqn=fqn,
+        unique_id=f'report.{pkg}.{name}',
+        package_name=pkg,
+        path=path,
+        root_path='/usr/src/app',
+        original_file_path=path,
+        owner=owner,
+    )
+
+
 @pytest.fixture
 def seed():
     return make_seed(
@@ -449,7 +476,7 @@ def manifest(seed, source, ephemeral_model, view_model, table_model, ext_source,
 
 
 def search_manifest_using_method(manifest, method, selection):
-    selected = method.search(set(manifest.nodes) | set(manifest.sources), selection)
+    selected = method.search(set(manifest.nodes) | set(manifest.sources) | set(manifest.reports), selection)
     results = {manifest.expect(uid).search_name for uid in selected}
     return results
 
@@ -559,6 +586,16 @@ def test_select_test_type(manifest):
     assert search_manifest_using_method(manifest, method, 'data') == {'view_test_nothing'}
 
 
+def test_select_report(manifest):
+    report = make_report('test', 'my_report')
+    manifest.reports[report.unique_id] = report
+    methods = MethodManager(manifest, None)
+    method = methods.get_method('report', [])
+    assert isinstance(method, ReportSelectorMethod)
+    assert search_manifest_using_method(manifest, method, 'my_report') == {'my_report'}
+    assert not search_manifest_using_method(manifest, method, 'not_my_report')
+
+
 @pytest.fixture
 def previous_state(manifest):
     writable = copy.deepcopy(manifest).writable_manifest()