Make filenames defined in action config files be treated as relative …

…to the action config file (#1636) * Update file name to be read as absolute * Fix tests
dataform-co · Jan 4, 2024 · 63f042c · 63f042c
1 parent 9c18d69
commit 63f042c
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 40 deletions.
diff --git a/core/main.ts b/core/main.ts
@@ -113,17 +113,7 @@ function loadActionConfigs(session: Session, filePaths: string[]) {
     )
     .sort()
     .forEach(actionConfigsPath => {
-      let actionConfigsAsJson = {};
-      try {
-        // tslint:disable-next-line: tsr-detect-non-literal-require
-        actionConfigsAsJson = nativeRequire(actionConfigsPath).asJson();
-      } catch (e) {
-        session.compileError(e, actionConfigsPath);
-      }
-      // TODO(ekrekr): Throw nice errors if the proto is invalid.
-      verifyObjectMatchesProto(dataform.ActionConfigs, actionConfigsAsJson);
-      const actionConfigs = dataform.ActionConfigs.fromObject(actionConfigsAsJson);
-
+      const actionConfigs = loadActionConfigsFile(session, actionConfigsPath);
       actionConfigs.actions.forEach(nonProtoActionConfig => {
         const actionConfig = dataform.ActionConfig.create(nonProtoActionConfig);
 
@@ -147,29 +137,54 @@ function loadActionConfigs(session: Session, filePaths: string[]) {
           actionConfig.target.name = fileNameAsTargetName;
         }
 
+        // Users define file paths relative to action config path, but internally and in the
+        // compiled graph they are absolute paths.
+        actionConfig.fileName =
+          actionConfigsPath.substring(0, actionConfigsPath.length - "actions.yaml".length) +
+          actionConfig.fileName;
+
         if (fileExtension === "ipynb") {
           const notebookContents = nativeRequire(actionConfig.fileName).asBase64String();
           session.notebook(actionConfig, notebookContents);
         }
 
         if (fileExtension === "sql") {
-          const queryAsContextable = nativeRequire(actionConfig.fileName).queryAsContextable;
-
-          if (actionConfig.assertion) {
-            return session.assertion(actionConfig, queryAsContextable);
-          }
-          if (actionConfig.table) {
-            return session.table(actionConfig, queryAsContextable);
-          }
-          if (actionConfig.incrementalTable) {
-            return session.incrementalTable(actionConfig, queryAsContextable);
-          }
-          if (actionConfig.view) {
-            return session.view(actionConfig, queryAsContextable);
-          }
-          // If no config is specified, the operation action type is defaulted to.
-          session.operate(actionConfig, queryAsContextable);
+          loadSqlFile(session, actionConfig);
         }
       });
     });
 }
+
+function loadActionConfigsFile(
+  session: Session,
+  actionConfigsPath: string
+): dataform.ActionConfigs {
+  let actionConfigsAsJson = {};
+  try {
+    // tslint:disable-next-line: tsr-detect-non-literal-require
+    actionConfigsAsJson = nativeRequire(actionConfigsPath).asJson();
+  } catch (e) {
+    session.compileError(e, actionConfigsPath);
+  }
+  verifyObjectMatchesProto(dataform.ActionConfigs, actionConfigsAsJson);
+  return dataform.ActionConfigs.fromObject(actionConfigsAsJson);
+}
+
+function loadSqlFile(session: Session, actionConfig: dataform.ActionConfig) {
+  const queryAsContextable = nativeRequire(actionConfig.fileName).queryAsContextable;
+
+  if (actionConfig.assertion) {
+    return session.assertion(actionConfig, queryAsContextable);
+  }
+  if (actionConfig.table) {
+    return session.table(actionConfig, queryAsContextable);
+  }
+  if (actionConfig.incrementalTable) {
+    return session.incrementalTable(actionConfig, queryAsContextable);
+  }
+  if (actionConfig.view) {
+    return session.view(actionConfig, queryAsContextable);
+  }
+  // If no config is specified, the operation action type is defaulted to.
+  session.operate(actionConfig, queryAsContextable);
+}
diff --git a/core/main_test.ts b/core/main_test.ts
@@ -541,7 +541,7 @@ select 1 AS \${dataform.projectConfig.vars.columnVar}`
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-  - fileName: definitions/notebook.ipynb`
+  - fileName: notebook.ipynb`
       );
 
       return projectDir;
@@ -638,8 +638,8 @@ actions:
     });
   });
 
-  suite("SQL actions", () => {
-    test(`SQL actions can be loaded via an actions config file`, () => {
+  suite("action configs", () => {
+    test(`SQL actions can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -653,7 +653,7 @@ actions:
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-  - fileName: definitions/action.sql`
+  - fileName: action.sql`
       );
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -695,7 +695,7 @@ actions:
       );
     });
 
-    test(`declarations can be loaded via an actions config file`, () => {
+    test(`declarations can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -747,7 +747,7 @@ actions:
       );
     });
 
-    test(`tables can be loaded via an actions config file`, () => {
+    test(`tables can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -761,7 +761,7 @@ actions:
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-  - fileName: definitions/action.sql
+  - fileName: action.sql
     table: {}`
       );
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
@@ -808,7 +808,7 @@ actions:
       );
     });
 
-    test(`incremental tables can be loaded via an actions config file`, () => {
+    test(`incremental tables can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -822,7 +822,7 @@ actions:
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-  - fileName: definitions/action.sql
+  - fileName: action.sql
     incrementalTable:
       protected: true
       uniqueKey:
@@ -878,7 +878,7 @@ actions:
       );
     });
 
-    test(`views can be loaded via an actions config file`, () => {
+    test(`views can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -892,7 +892,7 @@ actions:
         path.join(projectDir, "definitions/actions.yaml"),
         `
 actions:
-  - fileName: definitions/action.sql
+  - fileName: action.sql
     view: {}`
       );
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
@@ -939,7 +939,7 @@ actions:
       );
     });
 
-    test(`assertions can be loaded via an actions config file`, () => {
+    test(`assertions can be loaded`, () => {
       const projectDir = tmpDirFixture.createNewTmpDir();
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(
@@ -954,7 +954,7 @@ actions:
         `
 actions:
   - assertion: {}
-    fileName: definitions/action.sql`
+    fileName: action.sql`
       );
       // tslint:disable-next-line: tsr-detect-non-literal-fs-filename
       fs.writeFileSync(