Fix notebook dependency targets (#1792)

* Add better tests, fix proto conversion from config to compiled graph

* Tidy, fix some tests

* Extract target conversion for pure targets and action configs to separate functions, to fix include on dependency assertions

* Tidy

* Fix lint

* Force check retry

Author: Elias Kassell

core/actions/assertion.ts

@@ -5,6 +5,7 @@ import * as Path from "df/core/path";
 import { Session } from "df/core/session";
 import {
   actionConfigToCompiledGraphTarget,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolvableAsTarget,
   resolveActionsConfigFilename,
@@ -78,7 +79,7 @@ export class Assertion extends ActionBuilder<dataform.Assertion> {
     if (config.dependencyTargets) {
       this.dependencies(
         config.dependencyTargets.map(dependencyTarget =>
-          actionConfigToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
         )
       );
     }

core/actions/data_preparation.ts

@@ -4,7 +4,9 @@ import { Resolvable } from "df/core/common";
 import * as Path from "df/core/path";
 import { Session } from "df/core/session";
 import {
+  actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolveActionsConfigFilename
 } from "df/core/utils";
@@ -37,22 +39,27 @@ export class DataPreparation extends ActionBuilder<dataform.DataPreparation> {
     this.proto.dataPreparation = dataPreparationDefinition;
 
     // Find targets
-    const targets = getTargets(dataPreparationDefinition)
-    this.proto.targets = targets.map((target) => this.applySessionToTarget(
-      target,
-      session.projectConfig,
-      config.filename,
-      true
-    ));
-    this.proto.canonicalTargets = targets.map((target) => this.applySessionToTarget(target, session.canonicalProjectConfig));
+    const targets = getTargets(dataPreparationDefinition);
+    this.proto.targets = targets.map(target =>
+      this.applySessionToTarget(target, session.projectConfig, config.filename, true)
+    );
+    this.proto.canonicalTargets = targets.map(target =>
+      this.applySessionToTarget(target, session.canonicalProjectConfig)
+    );
 
     // Set the unique target key as the first target defined.
     // TODO: Remove once multiple targets are supported.
     this.proto.target = this.proto.targets[0];
     this.proto.canonicalTarget = this.proto.canonicalTargets[0];
 
     this.proto.tags = config.tags;
-    this.dependencies(config.dependencyTargets);
+    if (config.dependencyTargets) {
+      this.dependencies(
+        config.dependencyTargets.map(dependencyTarget =>
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+        )
+      );
+    }
     this.proto.fileName = config.filename;
     if (config.disabled) {
       this.proto.disabled = config.disabled;
@@ -72,7 +79,7 @@ export class DataPreparation extends ActionBuilder<dataform.DataPreparation> {
   public dependencies(value: Resolvable | Resolvable[]) {
     const newDependencies = Array.isArray(value) ? value : [value];
     newDependencies.forEach(resolvable =>
-        addDependenciesToActionDependencyTargets(this, resolvable)
+      addDependenciesToActionDependencyTargets(this, resolvable)
     );
     return this;
   }
@@ -101,17 +108,18 @@ export class DataPreparation extends ActionBuilder<dataform.DataPreparation> {
   }
 }
 
-function parseDataPreparationDefinitionJson(
-    dataPreparationAsJson: { [key: string]: unknown }): dataform.DataPreparationDefinition  {
+function parseDataPreparationDefinitionJson(dataPreparationAsJson: {
+  [key: string]: unknown;
+}): dataform.DataPreparationDefinition {
   try {
     return dataform.DataPreparationDefinition.create(
-        verifyObjectMatchesProto(
-            dataform.DataPreparationDefinition,
-            dataPreparationAsJson as {
-              [key: string]: any;
-            },
-            VerifyProtoErrorBehaviour.SHOW_DOCS_LINK
-        )
+      verifyObjectMatchesProto(
+        dataform.DataPreparationDefinition,
+        dataPreparationAsJson as {
+          [key: string]: any;
+        },
+        VerifyProtoErrorBehaviour.SHOW_DOCS_LINK
+      )
     );
   } catch (e) {
     if (e instanceof ReferenceError) {
@@ -121,20 +129,18 @@ function parseDataPreparationDefinitionJson(
   }
 }
 
-function getTargets(
-  definition: dataform.DataPreparationDefinition
-): dataform.Target[] {
+function getTargets(definition: dataform.DataPreparationDefinition): dataform.Target[] {
   const targets: dataform.Target[] = [];
 
   definition.nodes.forEach(node => {
     const table = node.destination?.table;
     if (table) {
       const compiledGraphTarget: dataform.ITarget = {
         database: table.project,
-        schema:table.dataset,
+        schema: table.dataset,
         name: table.table
       };
-      targets.push(dataform.Target.create(compiledGraphTarget))
+      targets.push(dataform.Target.create(compiledGraphTarget));
     }
   });
 

core/actions/incremental_table.ts

@@ -9,6 +9,7 @@ import { Session } from "df/core/session";
 import {
   actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolvableAsTarget,
   resolveActionsConfigFilename,
@@ -104,7 +105,11 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
       this.setDependOnDependencyAssertions(config.dependOnDependencyAssertions);
     }
     if (config.dependencyTargets) {
-      this.dependencies(config.dependencyTargets);
+      this.dependencies(
+        config.dependencyTargets.map(dependencyTarget =>
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+        )
+      );
     }
     if (config.hermetic !== undefined) {
       this.hermetic(config.hermetic);

core/actions/notebook.ts

@@ -6,6 +6,7 @@ import { Session } from "df/core/session";
 import {
   actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolveActionsConfigFilename
 } from "df/core/utils";
@@ -46,7 +47,13 @@ export class Notebook extends ActionBuilder<dataform.Notebook> {
     this.proto.canonicalTarget = this.applySessionToTarget(target, session.canonicalProjectConfig);
     this.proto.tags = config.tags;
     this.dependOnDependencyAssertions = config.dependOnDependencyAssertions;
-    this.dependencies(config.dependencyTargets);
+    if (config.dependencyTargets) {
+      this.dependencies(
+        config.dependencyTargets.map(dependencyTarget =>
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+        )
+      );
+    }
     this.proto.fileName = config.filename;
     if (config.disabled) {
       this.proto.disabled = config.disabled;

core/actions/operation.ts

@@ -7,6 +7,7 @@ import { Session } from "df/core/session";
 import {
   actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolvableAsTarget,
   resolveActionsConfigFilename,
@@ -77,7 +78,7 @@ export class Operation extends ActionBuilder<dataform.Operation> {
     if (config.dependencyTargets) {
       this.dependencies(
         config.dependencyTargets.map(dependencyTarget =>
-          actionConfigToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
         )
       );
     }

core/actions/table.ts

@@ -19,6 +19,7 @@ import {
   actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
   checkExcessProperties,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolvableAsTarget,
   resolveActionsConfigFilename,
@@ -345,7 +346,7 @@ export class Table extends ActionBuilder<dataform.Table> {
       this.config({
         type: "table",
         dependencies: config.dependencyTargets.map(dependencyTarget =>
-          actionConfigToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
         ),
         tags: config.tags,
         disabled: config.disabled,
@@ -377,7 +378,7 @@ export class Table extends ActionBuilder<dataform.Table> {
       this.config({
         type: "view",
         dependencies: config.dependencyTargets.map(dependencyTarget =>
-          actionConfigToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
         ),
         disabled: config.disabled,
         materialized: config.materialized,
@@ -431,7 +432,7 @@ export class Table extends ActionBuilder<dataform.Table> {
       this.config({
         type: "incremental",
         dependencies: config.dependencyTargets.map(dependencyTarget =>
-          actionConfigToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
         ),
         disabled: config.disabled,
         protected: config.protected,

core/actions/view.ts

@@ -9,6 +9,7 @@ import { Session } from "df/core/session";
 import {
   actionConfigToCompiledGraphTarget,
   addDependenciesToActionDependencyTargets,
+  configTargetToCompiledGraphTarget,
   nativeRequire,
   resolvableAsTarget,
   resolveActionsConfigFilename,
@@ -96,7 +97,11 @@ export class View extends ActionBuilder<dataform.Table> {
       this.setDependOnDependencyAssertions(config.dependOnDependencyAssertions);
     }
     if (config.dependencyTargets) {
-      this.dependencies(config.dependencyTargets);
+      this.dependencies(
+        config.dependencyTargets.map(dependencyTarget =>
+          configTargetToCompiledGraphTarget(dataform.ActionConfig.Target.create(dependencyTarget))
+        )
+      );
     }
     if (config.hermetic !== undefined) {
       this.hermetic(config.hermetic);