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

…nerate"
dbt-labs · Oct 7, 2019 · 4dd0d9d · 4dd0d9d
1 parent 5061397
commit 4dd0d9d
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 8 deletions.
diff --git a/core/dbt/task/generate.py b/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))

diff --git a/core/dbt/task/runnable.py b/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
 
 

diff --git a/test/integration/048_rpc_test/test_rpc.py b/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):