PR feedback, also remove an extra manifest-building from "dbt docs ge…

…nerate"

core/dbt/task/generate.py

@@ -19,6 +19,7 @@
 import dbt.exceptions
 
 from dbt.task.compile import CompileTask
+from dbt.task.runnable import write_manifest
 
 
 CATALOG_FILENAME = 'catalog.json'
@@ -178,8 +179,8 @@ def _coerce_decimal(value):
 
 class GenerateTask(CompileTask):
  def _get_manifest(self) -> Manifest:
- manifest = dbt.loader.GraphLoader.load_all(self.config)
- return manifest
+ # manifest = dbt.loader.GraphLoader.load_all(self.config)
+ return self.manifest
 
  def run(self):
  compile_results = None
@@ -197,10 +198,8 @@ def run(self):
 
  adapter = get_adapter(self.config)
  with adapter.connection_named('generate_catalog'):
- manifest = self._get_manifest()
-
  dbt.ui.printer.print_timestamped_line("Building catalog")
- catalog_table = adapter.get_catalog(manifest)
+ catalog_table = adapter.get_catalog(self.manifest)
 
  catalog_data: List[PrimitiveDict] = [
  dict(zip(catalog_table.column_names, map(_coerce_decimal, row)))
@@ -209,13 +208,14 @@ def run(self):
 
  catalog = Catalog(catalog_data)
  results = self.get_catalog_results(
- nodes=catalog.make_unique_id_map(manifest),
+ nodes=catalog.make_unique_id_map(self.manifest),
  generated_at=datetime.utcnow(),
  compile_results=compile_results,
  )
 
  path = os.path.join(self.config.target_path, CATALOG_FILENAME)
  results.write(path)
+ write_manifest(self.manifest, self.config)
 
  dbt.ui.printer.print_timestamped_line(
  'Catalog written to {}'.format(os.path.abspath(path))

core/dbt/task/runnable.py

@@ -21,6 +21,11 @@
 MANIFEST_FILE_NAME = 'manifest.json'
 
 
+def write_manifest(manifest, config):
+ if dbt.flags.WRITE_JSON:
+ manifest.write(os.path.join(config.target_path, MANIFEST_FILE_NAME))
+
+
 def load_manifest(config):
  # performance trick: if the adapter has a manifest loaded, use that to
  # avoid parsing internal macros twice. Also, when loading the adapter's
@@ -31,8 +36,7 @@ def load_manifest(config):
  internal = adapter.load_internal_manifest()
  manifest = GraphLoader.load_all(config, internal_manifest=internal)
 
- if dbt.flags.WRITE_JSON:
- manifest.write(os.path.join(config.target_path, MANIFEST_FILE_NAME))
+ write_manifest(manifest, config)
  return manifest
 
 

test/integration/029_docs_generate_tests/test_docs_generate.py

@@ -821,7 +821,7 @@ def expected_seeded_manifest(self, model_database=None):
  return {
  'nodes': {
  'model.test.model': {
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/model.sql'),
  'name': 'model',
  'root_path': self.test_root_dir,
  'resource_type': 'model',
@@ -869,9 +869,17 @@ def expected_seeded_manifest(self, model_database=None):
  },
  'patch_path': schema_yml_path,
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'seed.test.seed': {
  'build_path': None,
+ 'compiled': True,
+ 'compiled_sql': '',
  'config': {
  'enabled': True,
  'materialized': 'seed',
@@ -905,10 +913,16 @@ def expected_seeded_manifest(self, model_database=None):
  'description': '',
  'columns': {},
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': '',
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': '',
+ 'wrapped_sql': 'None',
  },
  'test.test.not_null_model_id': {
  'alias': 'not_null_model_id',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/schema_test/not_null_model_id.sql'),
  'column_name': 'id',
  'columns': {},
  'config': {
@@ -941,10 +955,16 @@ def expected_seeded_manifest(self, model_database=None):
  'tags': ['schema'],
  'unique_id': 'test.test.not_null_model_id',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': AnyStringWith('count(*)'),
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': AnyStringWith('count(*)'),
+ 'wrapped_sql': AnyStringWith('count(*)'),
  },
  'test.test.test_nothing_model_': {
  'alias': 'test_nothing_model_',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/schema_test/test_nothing_model_.sql'),
  'column_name': None,
  'columns': {},
  'config': {
@@ -977,10 +997,16 @@ def expected_seeded_manifest(self, model_database=None):
  'tags': ['schema'],
  'unique_id': 'test.test.test_nothing_model_',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': AnyStringWith('select 0'),
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': AnyStringWith('select 0'),
+ 'wrapped_sql': AnyStringWith('select 0'),
  },
  'test.test.unique_model_id': {
  'alias': 'unique_model_id',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/schema_test/unique_model_id.sql'),
  'column_name': 'id',
  'columns': {},
  'config': {
@@ -1013,6 +1039,12 @@ def expected_seeded_manifest(self, model_database=None):
  'tags': ['schema'],
  'unique_id': 'test.test.unique_model_id',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': AnyStringWith('count(*)'),
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': AnyStringWith('count(*)'),
+ 'wrapped_sql': AnyStringWith('count(*)'),
  },
  },
  'parent_map': {
@@ -1123,7 +1155,7 @@ def expected_postgres_references_manifest(self, model_database=None):
  'nodes': {
  'model.test.ephemeral_copy': {
  'alias': 'ephemeral_copy',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/ephemeral_copy.sql'),
  'columns': {},
  'config': {
  'column_types': {},
@@ -1160,10 +1192,16 @@ def expected_postgres_references_manifest(self, model_database=None):
  'database': self.default_database,
  'tags': [],
  'unique_id': 'model.test.ephemeral_copy',
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'model.test.ephemeral_summary': {
  'alias': 'ephemeral_summary',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/ephemeral_summary.sql'),
  'columns': {
  'first_name': {
  'description': 'The first name being summarized',
@@ -1228,10 +1266,17 @@ def expected_postgres_references_manifest(self, model_database=None):
  'schema': my_schema_name,
  'database': self.default_database,
  'tags': [],
- 'unique_id': 'model.test.ephemeral_summary'},
+ 'unique_id': 'model.test.ephemeral_summary',
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [ANY],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
+ },
  'model.test.view_summary': {
  'alias': 'view_summary',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/view_summary.sql'),
  'columns': {
  'first_name': {
  'description': 'The first name being summarized',
@@ -1295,7 +1340,13 @@ def expected_postgres_references_manifest(self, model_database=None):
  'schema': my_schema_name,
  'sources': [],
  'tags': [],
- 'unique_id': 'model.test.view_summary'
+ 'unique_id': 'model.test.view_summary',
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'seed.test.seed': {
  'alias': 'seed',
@@ -1330,7 +1381,13 @@ def expected_postgres_references_manifest(self, model_database=None):
  'schema': my_schema_name,
  'database': self.default_database,
  'tags': [],
- 'unique_id': 'seed.test.seed'
+ 'unique_id': 'seed.test.seed',
+ 'compiled': True,
+ 'compiled_sql': '',
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': '',
+ 'wrapped_sql': 'None',
  },
  'source.test.my_source.my_table': {
  'columns': {
@@ -1365,7 +1422,7 @@ def expected_postgres_references_manifest(self, model_database=None):
  },
  ],
  'external': {
- 'file_format': None, 'location': None, 'partitions': None, 
+ 'file_format': None, 'location': None, 'partitions': None,
  'row_format': None, 'tbl_properties': None
  },
  'freshness': {'error_after': None, 'warn_after': None, 'filter': None},
@@ -1589,7 +1646,7 @@ def expected_bigquery_complex_manifest(self):
  'sources': [],
  'depends_on': {'macros': [], 'nodes': ['seed.test.seed']},
  'fqn': ['test', 'clustered'],
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/clustered.sql'),
  'name': 'clustered',
  'original_file_path': clustered_sql_path,
  'package_name': 'test',
@@ -1632,10 +1689,16 @@ def expected_bigquery_complex_manifest(self):
  'description': 'A clustered and partitioned copy of the test model',
  'patch_path': self.dir('bq_models/schema.yml'),
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'model.test.multi_clustered': {
  'alias': 'multi_clustered',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/multi_clustered.sql'),
  'config': {
  'cluster_by': ['first_name', 'email'],
  'column_types': {},
@@ -1694,10 +1757,16 @@ def expected_bigquery_complex_manifest(self):
  'description': 'A clustered and partitioned copy of the test model, clustered on multiple columns',
  'patch_path': self.dir('bq_models/schema.yml'),
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'model.test.nested_view': {
  'alias': 'nested_view',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/nested_view.sql'),
  'config': {
  'column_types': {},
  'enabled': True,
@@ -1757,10 +1826,16 @@ def expected_bigquery_complex_manifest(self):
  'description': 'The test model',
  'patch_path': self.dir('bq_models/schema.yml'),
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'model.test.nested_table': {
  'alias': 'nested_table',
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/nested_table.sql'),
  'config': {
  'column_types': {},
  'enabled': True,
@@ -1794,6 +1869,12 @@ def expected_bigquery_complex_manifest(self):
  'columns': {},
  'description': '',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'seed.test.seed': {
  'build_path': None,
@@ -1832,6 +1913,12 @@ def expected_bigquery_complex_manifest(self):
  'columns': {},
  'description': '',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': '',
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': '',
+ 'wrapped_sql': 'None',
  },
  },
  'child_map': {
@@ -1960,7 +2047,7 @@ def expected_redshift_incremental_view_manifest(self):
  return {
  'nodes': {
  'model.test.model': {
- 'build_path': None,
+ 'build_path': os.path.normpath('target/compiled/test/model.sql'),
  'name': 'model',
  'root_path': self.test_root_dir,
  'resource_type': 'model',
@@ -2022,6 +2109,12 @@ def expected_redshift_incremental_view_manifest(self):
  },
  'patch_path': self.dir('rs_models/schema.yml'),
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  'seed.test.seed': {
  'build_path': None,
@@ -2060,6 +2153,12 @@ def expected_redshift_incremental_view_manifest(self):
  'columns': {},
  'description': '',
  'docrefs': [],
+ 'compiled': True,
+ 'compiled_sql': ANY,
+ 'extra_ctes_injected': True,
+ 'extra_ctes': [],
+ 'injected_sql': ANY,
+ 'wrapped_sql': 'None',
  },
  },
  'parent_map': {

test/integration/048_rpc_test/test_rpc.py

@@ -927,6 +927,8 @@ def test_docs_generate_postgres(self):
  self.run_dbt_with_vars(['seed'])
  self.run_dbt_with_vars(['run'])
  self.assertFalse(os.path.exists('target/catalog.json'))
+ if os.path.exists('target/manifest.json'):
+ os.remove('target/manifest.json')
  result = self.async_query('docs.generate').json()
  dct = self.assertIsResult(result)
  self.assertTrue(os.path.exists('target/catalog.json'))
@@ -949,6 +951,11 @@ def test_docs_generate_postgres(self):
  }
  for uid in expected:
  self.assertIn(uid, nodes)
+ self.assertTrue(os.path.exists('target/manifest.json'))
+ with open('target/manifest.json') as fp:
+ manifest = json.load(fp)
+ self.assertIn('nodes', manifest)
+ self.assertEqual(len(manifest['nodes']), 17)
 
 
 class FailedServerProcess(ServerProcess):