mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix(pipelines): // macros takes precedence over // pipeline (a library is never a member)
A macro library is definition-only — its macros are injected into consumers and running it is a no-op — so marking it `// pipeline` is meaningless and only produced a confusing state (an unused pipeline macro library appearing as a manual root). Make `// macros` win: parse_pipeline_annotations forces in_pipeline false when macros is set. Mirrored in all three parsers that must agree — the Rust canonical parser (drives deploy membership), the frontend TS parser (live graph), and the CLI local graph (pinned wasm still reports in_pipeline, so precedence is applied when skipping members). Shared parity fixture + unit tests on each side.
This commit is contained in:
@@ -457,6 +457,11 @@ export async function buildLocalPipelineGraph(args: {
|
||||
const folderDir = path.join(args.root, "f", folderClean);
|
||||
const all = await collectScripts(folderDir, args.root, args.defaultTs);
|
||||
|
||||
// `// macros` DuckDB libraries across the whole workspace (a pipeline may use a
|
||||
// shared library outside its folder). `// macros` wins over `// pipeline`, so a
|
||||
// library is never a pipeline member — matching the backend parser precedence.
|
||||
const libMacros = collectMacroLibraries(args.root);
|
||||
|
||||
const runnables: GraphRunnable[] = [];
|
||||
const edges: GraphEdge[] = [];
|
||||
const triggers: GraphTrigger[] = [];
|
||||
@@ -472,7 +477,9 @@ export async function buildLocalPipelineGraph(args: {
|
||||
|
||||
for (const s of all) {
|
||||
const out = await inferScriptAssets(s.content, s.language);
|
||||
if (!out.in_pipeline) continue; // not a pipeline member
|
||||
// Not a member: no `// pipeline` marker, OR a `// macros` library (which the
|
||||
// pinned wasm still reports as `in_pipeline` — apply the precedence here).
|
||||
if (!out.in_pipeline || libMacros.has(s.path)) continue;
|
||||
const retry = normalizeRetry(out.retry);
|
||||
const nativeTriggers = recoverHeaderNativeTriggers(s.content, s.language);
|
||||
// Carry the parsed `// tag` so previews route to the same worker the
|
||||
@@ -599,15 +606,6 @@ export async function buildLocalPipelineGraph(args: {
|
||||
}
|
||||
}
|
||||
|
||||
// `// macros` libraries + lib→consumer edges. The wasm asset parser drops the
|
||||
// `// macros` / `// use` annotations and never emits a macro registry, so we
|
||||
// derive both from the working tree here (see ./duckdbMacros.ts) to match the
|
||||
// deployed graph, which records them at deploy. Libraries are discovered
|
||||
// WORKSPACE-WIDE (not just this folder) — the deployed builder fetches the
|
||||
// macro registry unfiltered so a shared library (e.g. `f/shared/stats`) is the
|
||||
// provider endpoint of an in-folder consumer's edge; only consumers are
|
||||
// folder-scoped. Macros are DuckDB-only.
|
||||
const libMacros = collectMacroLibraries(args.root);
|
||||
const macroEdges = buildMacroEdges(all, libMacros, runnables);
|
||||
|
||||
const assets = [...assetSet.entries()].map(([key, a]) => {
|
||||
@@ -665,13 +663,10 @@ function collectMacroLibraries(root: string): Map<string, ParsedMacro[]> {
|
||||
return out;
|
||||
}
|
||||
|
||||
// Resolve which of this folder's pipeline scripts call the workspace's macro
|
||||
// libraries (by lexical call detection + `// use` annotations), then (a) mutate
|
||||
// `runnables` to add each referenced library as a node carrying its macro
|
||||
// signatures, and (b) return the lib→consumer edges. Mirrors the deployed graph
|
||||
// builder (`asset_graph` in windmill-api-assets): libraries come from the
|
||||
// workspace-wide registry, consumers are folder-scoped, and a library node
|
||||
// appears only when it is an endpoint of at least one edge (unused → not shown).
|
||||
// Derive lib→consumer edges (lexical calls + `// use`) and add each referenced
|
||||
// library to `runnables` as a node with its signatures. Mirrors the deployed
|
||||
// `asset_graph`: libraries are workspace-wide, consumers folder-scoped, and an
|
||||
// unused library (no edge) is not surfaced.
|
||||
function buildMacroEdges(
|
||||
all: LocalScript[],
|
||||
libMacros: Map<string, ParsedMacro[]>,
|
||||
@@ -755,28 +750,17 @@ function buildMacroEdges(
|
||||
a.consumer_path.localeCompare(b.consumer_path),
|
||||
);
|
||||
|
||||
// Tag library nodes with their macro signatures, like the deployed builder
|
||||
// (which sets `macros` on any node whose path provides macros, edge or not).
|
||||
// Two sources:
|
||||
// • every edge provider — added as a node if it isn't already one; and
|
||||
// • every library that is ALREADY a runnable (a `// pipeline` + `// macros`
|
||||
// script), even with no consumers yet — so it's recognized as
|
||||
// definition-only and never scheduled as a manual root.
|
||||
// An unused NON-pipeline library stays absent (suppressed), matching deployed.
|
||||
// A library node surfaces only when it is an edge provider (a consumer uses
|
||||
// it); an unused library is not shown. Libraries are never pipeline members
|
||||
// (`// macros` wins over `// pipeline`), so each is a fresh node here.
|
||||
const libPaths = new Set<string>(edges.map((e) => e.lib_path));
|
||||
const existingPaths = new Set(runnables.map((r) => r.path));
|
||||
for (const lib of libMacros.keys()) {
|
||||
if (existingPaths.has(lib)) libPaths.add(lib);
|
||||
}
|
||||
for (const lib of libPaths) {
|
||||
const macros = (libMacros.get(lib) ?? []).map((m) => ({
|
||||
name: m.name,
|
||||
params: m.params,
|
||||
is_table: m.isTable,
|
||||
}));
|
||||
const existing = runnables.find((r) => r.path === lib);
|
||||
if (existing) existing.macros = macros;
|
||||
else runnables.push({ path: lib, usage_kind: "script", macros });
|
||||
runnables.push({ path: lib, usage_kind: "script", macros });
|
||||
}
|
||||
// Force every edge's CONSUMER endpoint into the node set too, like the deployed
|
||||
// builder, so no edge dangles at a missing runnable. A consumer that is itself
|
||||
|
||||
@@ -526,25 +526,48 @@ test("a non-pipeline DuckDB macro consumer is a display-only node, never a run s
|
||||
);
|
||||
});
|
||||
|
||||
test("an UNUSED `// pipeline` + `// macros` library is still tagged with its macros", async () => {
|
||||
// A DuckDB file can be both `// pipeline` and `// macros`. The deployed builder
|
||||
// tags such a node with its macros even when no consumer uses it yet, so it's
|
||||
// recognized as definition-only and never scheduled as a manual root. Local
|
||||
// enrichment must do the same (otherwise `run --local` would run it).
|
||||
test("`// macros` wins over `// pipeline`: the library is never a pipeline member", async () => {
|
||||
// A DuckDB file marked both `// pipeline` and `// macros` is treated as a
|
||||
// definition-only library (mirrors the backend parser precedence). So an
|
||||
// UNUSED such library is suppressed exactly like a plain `// macros` one — not
|
||||
// a member node, hence never a run step.
|
||||
await withFolder(
|
||||
{
|
||||
"lib.duckdb.sql": `-- pipeline\n-- macros\nCREATE MACRO dbl(a) AS a * 2;\n`,
|
||||
"root.duckdb.sql": `-- pipeline\n-- materialize ducklake://main/out\nSELECT 1 AS v;\n`,
|
||||
},
|
||||
async (root, folder) => {
|
||||
const { graph } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" });
|
||||
// the library node carries its macro signatures despite having no consumers
|
||||
const { graph, scripts } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" });
|
||||
// unused library → suppressed (not a node), and never a previewable member
|
||||
expect(graph.runnables.map((r) => r.path)).toEqual(["f/mypipe/root"]);
|
||||
expect(scripts.map((s) => s.path)).toEqual(["f/mypipe/root"]);
|
||||
expect(graph.macro_edges).toBeUndefined();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("a USED `// pipeline` + `// macros` library appears as a library node (not a member)", async () => {
|
||||
await withFolder(
|
||||
{
|
||||
"lib.duckdb.sql": `-- pipeline\n-- macros\nCREATE MACRO dbl(a) AS a * 2;\n`,
|
||||
"root.duckdb.sql": `-- pipeline\n-- on datatable://main/t\nSELECT dbl(x) FROM main.t;\n`,
|
||||
},
|
||||
async (root, folder) => {
|
||||
const { graph, scripts } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" });
|
||||
// the library surfaces as a node with its signatures (it's used) …
|
||||
expect(graph.runnables.find((r) => r.path === "f/mypipe/lib")?.macros).toEqual([
|
||||
{ name: "dbl", params: "a", is_table: false },
|
||||
]);
|
||||
// no consumer → no macro edges, but the node is still marked (macros.length
|
||||
// > 0 is what `pipeline run` uses to exclude it from the run selection)
|
||||
expect(graph.macro_edges).toBeUndefined();
|
||||
// … but is NOT a pipeline member: absent from the previewable `scripts` set
|
||||
expect(scripts.map((s) => s.path)).toEqual(["f/mypipe/root"]);
|
||||
expect(graph.macro_edges).toEqual([
|
||||
{
|
||||
lib_path: "f/mypipe/lib",
|
||||
consumer_path: "f/mypipe/root",
|
||||
macro_names: ["dbl"],
|
||||
via_use: false,
|
||||
},
|
||||
]);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user