Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions core/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,51 @@ export function main(coreExecutionRequest: Uint8Array | string): Uint8Array | st
return dataform.CoreExecutionResponse.encode(coreExecutionResponse).finish();
}

// Every action type that accepts a `filename` in actions.yaml. Referencing a
// .sqlx file (including a data-preparation .dp.sqlx file) is unsupported: .sqlx
// files are compiled directly from the definitions/ directory by the sqlx
// compiler, not through the `actions.yaml` config. Without this check a .sqlx
// filename produces a cryptic `nativeRequire` error rather than a useful
// diagnostic, so we emit a clear compilation error up front. See issue #1785.
const ACTION_TYPES_REJECTING_SQLX_FILENAMES: string[] = [
"table",
"view",
"incrementalTable",
"assertion",
"operation",
"declaration",
"notebook",
"dataPreparation"
];

function actionConfigFilenameIsInvalidSqlx(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionConfigIncludesSqlxFiles?

session: Session,
actionConfig: dataform.ActionConfig,
actionConfigsPath: string
): boolean {
for (const actionType of ACTION_TYPES_REJECTING_SQLX_FILENAMES) {
const subConfig = (actionConfig as any)[actionType] as
| { filename?: string }
| undefined
| null;
Comment on lines +101 to +104
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't use dynamic casts to any and and just statically list all unsupported types following this pattern

const filename = subConfig?.filename;
if (filename && filename.toLowerCase().endsWith(".sqlx")) {
session.compileError(
new Error(
`Action config "${actionType}" has filename "${filename}", but .sqlx ` +
`files cannot be referenced from actions.yaml. .sqlx files are ` +
`compiled directly from the definitions/ directory. Either use a ` +
`.sql file with the same contents, or remove the actions.yaml ` +
`entry and let Dataform pick up the .sqlx file automatically.`
),
actionConfigsPath
);
return true;
}
}
return false;
}

function loadActionConfigs(session: Session, filePaths: string[]) {
filePaths
.filter(
Expand All @@ -89,6 +134,10 @@ function loadActionConfigs(session: Session, filePaths: string[]) {
actionConfigs.actions.forEach(nonProtoActionConfig => {
const actionConfig = dataform.ActionConfig.create(nonProtoActionConfig);

if (actionConfigFilenameIsInvalidSqlx(session, actionConfig, actionConfigsPath)) {
return;
}

if (actionConfig.table) {
session.actions.push(
new Table(
Expand Down
127 changes: 127 additions & 0 deletions core/main_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,133 @@ actions:`
);
});

suite(`.sqlx filenames in actions.yaml are rejected with a clear error`, () => {
[
{
actionType: "table",
yamlBody: `
actions:
- table:
filename: table.sqlx`
},
{
actionType: "view",
yamlBody: `
actions:
- view:
filename: view.sqlx`
},
{
actionType: "incrementalTable",
yamlBody: `
actions:
- incrementalTable:
filename: incremental.sqlx`
},
{
actionType: "assertion",
yamlBody: `
actions:
- assertion:
filename: assertion.sqlx`
},
{
actionType: "operation",
yamlBody: `
actions:
- operation:
filename: operation.sqlx`
},
{
actionType: "declaration",
yamlBody: `
actions:
- declaration:
name: declaration
filename: declaration.sqlx`
},
{
actionType: "notebook",
yamlBody: `
actions:
- notebook:
filename: notebook.sqlx`
}
].forEach(({ actionType, yamlBody }) => {
test(`for action type "${actionType}"`, () => {
const projectDir = tmpDirFixture.createNewTmpDir();
fs.writeFileSync(
path.join(projectDir, "workflow_settings.yaml"),
VALID_WORKFLOW_SETTINGS_YAML
);
fs.mkdirSync(path.join(projectDir, "definitions"));
fs.writeFileSync(path.join(projectDir, "definitions/actions.yaml"), yamlBody);

const result = runMainInVm(coreExecutionRequestFromPath(projectDir));

const errorMessages = result.compile.compiledGraph.graphErrors.compilationErrors.map(
({ message }) => message
);
expect(errorMessages).to.have.lengthOf(1);
expect(errorMessages[0]).to.include(`Action config "${actionType}" has filename`);
expect(errorMessages[0]).to.include(
".sqlx files cannot be referenced from actions.yaml"
);
});
});

test(`is case-insensitive`, () => {
const projectDir = tmpDirFixture.createNewTmpDir();
fs.writeFileSync(
path.join(projectDir, "workflow_settings.yaml"),
VALID_WORKFLOW_SETTINGS_YAML
);
fs.mkdirSync(path.join(projectDir, "definitions"));
fs.writeFileSync(
path.join(projectDir, "definitions/actions.yaml"),
`
actions:
- table:
filename: table.SQLX`
);

const result = runMainInVm(coreExecutionRequestFromPath(projectDir));

expect(
result.compile.compiledGraph.graphErrors.compilationErrors.map(({ message }) => message)
).to.have.lengthOf(1);
});

test(`for data preparations referencing a .dp.sqlx file`, () => {
// .dp.sqlx data preparations are compiled directly from the definitions/
// directory, so referencing one from actions.yaml is the same mistake as
// for any other action type and must produce the same clear error rather
// than silently loading a malformed, duplicated action.
const projectDir = tmpDirFixture.createNewTmpDir();
fs.writeFileSync(
path.join(projectDir, "workflow_settings.yaml"),
VALID_WORKFLOW_SETTINGS_YAML
);
fs.mkdirSync(path.join(projectDir, "definitions"));
fs.writeFileSync(
path.join(projectDir, "definitions/actions.yaml"),
`
actions:
- dataPreparation:
filename: prep.dp.sqlx`
);

const result = runMainInVm(coreExecutionRequestFromPath(projectDir));

const errorMessages = result.compile.compiledGraph.graphErrors.compilationErrors.map(
({ message }) => message
);
expect(errorMessages).to.have.lengthOf(1);
expect(errorMessages[0]).to.include(`Action config "dataPreparation" has filename`);
expect(errorMessages[0]).to.include(".sqlx files cannot be referenced from actions.yaml");
});
});

test(`filenames with non-UTF8 characters are valid`, () => {
const projectDir = tmpDirFixture.createNewTmpDir();
fs.writeFileSync(
Expand Down