diff --git a/frontend/src/lib/components/assets/AssetGraph/AddDataSourceMenu.svelte b/frontend/src/lib/components/assets/AssetGraph/AddDataSourceMenu.svelte index d53554a83e..49a222148b 100644 --- a/frontend/src/lib/components/assets/AssetGraph/AddDataSourceMenu.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/AddDataSourceMenu.svelte @@ -40,16 +40,13 @@ webhook, a queue…): the add node's, and the canvas's right-click one. trigger: Snippet<[{ open: boolean }]> /** Opens the menu each time it changes. */ openSignal?: number - /** With `openSignal`: the output the new script writes. */ - presetOutput?: { kind: 'ducklake' | 'datatable'; store: string; table: string } } let { onAddPipelineScript, pathPrefix, defaultPathSuffix, trigger, - openSignal, - presetOutput + openSignal }: Props = $props() function handlePick(pick: PipelineInsertPick) { @@ -149,6 +146,5 @@ webhook, a queue…): the add node's, and the canvas's right-click one. {defaultPathSuffix} onPick={handlePick} {openSignal} - {presetOutput} {trigger} /> diff --git a/frontend/src/lib/components/assets/AssetGraph/AssetGraphCanvas.svelte b/frontend/src/lib/components/assets/AssetGraph/AssetGraphCanvas.svelte index 951fa573d8..67dbc5b8bd 100644 --- a/frontend/src/lib/components/assets/AssetGraph/AssetGraphCanvas.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/AssetGraphCanvas.svelte @@ -365,9 +365,6 @@ }) let addMenuAt = $state<{ x: number; y: number }>({ x: 0, y: 0 }) let addMenuSignal = $state(0) - let addMenuPreset = $state< - { kind: 'ducklake' | 'datatable'; store: string; table: string } | undefined - >(undefined) // The schedule wizard a Fix action opens for an existing script, at the pill. let scheduleWizard = $state< @@ -391,11 +388,6 @@ if (a.scheduleFor) { scheduleWizard = { ...a.scheduleFor, at: anchor } scheduleWizardSignal++ - } else if (a.addWriter) { - const [store, ...rest] = a.addWriter.path.split('/') - addMenuAt = anchor - addMenuPreset = { kind: a.addWriter.kind, store, table: rest.join('/') } - addMenuSignal++ } else a.run?.() } })) @@ -1865,7 +1857,6 @@ onclick={() => { if (paneMenu) addMenuAt = paneMenu paneMenu = undefined - addMenuPreset = undefined addMenuSignal++ }} > @@ -1937,7 +1928,6 @@ {pathPrefix} {defaultPathSuffix} openSignal={addMenuSignal} - presetOutput={addMenuPreset} > {#snippet trigger()} diff --git a/frontend/src/lib/components/assets/AssetGraph/AssetNode.svelte b/frontend/src/lib/components/assets/AssetGraph/AssetNode.svelte index 2c7a0bdbc9..571b25ed28 100644 --- a/frontend/src/lib/components/assets/AssetGraph/AssetNode.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/AssetNode.svelte @@ -219,7 +219,8 @@ } } - let showAdd = $derived(data.onAddScript != undefined) + // A red asset needs fixing where its error comes from, not a downstream step. + let showAdd = $derived(data.onAddScript != undefined && !data.error) let menuItems: Item[] = $derived( data.onDeleteUpstream ? [ diff --git a/frontend/src/lib/components/assets/AssetGraph/NodeFixButton.svelte b/frontend/src/lib/components/assets/AssetGraph/NodeFixButton.svelte index 89112eca00..5744ef367c 100644 --- a/frontend/src/lib/components/assets/AssetGraph/NodeFixButton.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/NodeFixButton.svelte @@ -16,15 +16,14 @@ red fill already says something is wrong. export type FixExplainer = Array void }> export type NodeFix = { explainer: FixExplainer; actions: NodeFixAction[] } - /** A fix as its supplier declares it: an action, or opening the add menu with - * the new script's output preset (which the canvas owns). */ + /** A fix as its supplier declares it: an action, or opening the schedule + * wizard (which the canvas owns). */ export type NodeFixSpec = { explainer: FixExplainer actions: Array<{ label: string detail?: string run?: () => void - addWriter?: { kind: 'ducklake' | 'datatable'; path: string } /** Opens the schedule wizard for this script; its confirm calls `onSchedule`. */ scheduleFor?: { script: string diff --git a/frontend/src/lib/components/assets/AssetGraph/PipelineInsertMenu.svelte b/frontend/src/lib/components/assets/AssetGraph/PipelineInsertMenu.svelte index 6374526e51..56f0877ce0 100644 --- a/frontend/src/lib/components/assets/AssetGraph/PipelineInsertMenu.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/PipelineInsertMenu.svelte @@ -87,8 +87,6 @@ placement?: 'bottom' | 'top' | 'left' | 'right' /** Opens the menu each time it changes, for an entry point other than the trigger. */ openSignal?: number - /** With `openSignal`: the new script's output, already picked and named. */ - presetOutput?: { kind: 'ducklake' | 'datatable'; store: string; table: string } /** With `openSignal`: only the schedule step, for this existing script; its * confirm hands the schedule to `onSchedule` instead of creating a script. */ scheduleFor?: string @@ -104,7 +102,6 @@ defaultPathSuffix, onPick, openSignal, - presetOutput, scheduleFor, onSchedule }: Props = $props() @@ -122,21 +119,9 @@ }) let selected = $state(buildEmptySelected()) let menuOpen = $state(false) - // Opened for a given output: the output is not the user's to pick. - let outputLocked = $state(false) $effect(() => { if (!openSignal) return untrack(() => { - outputLocked = !!presetOutput - if (presetOutput) { - resetWizard() - selected.outputId = presetOutput.kind - config.asset = { ...presetOutput } - config.tableEdited = true - // Named after what it writes, in the characters a script path allows. - const name = presetOutput.table.replace(/[^\w-]+/g, '_').replace(/^_+|_+$/g, '') - if (name) selected.scriptPath = name - } if (scheduleFor) { resetWizard() selected.triggerId = 'schedule' @@ -340,14 +325,7 @@ contentClasses={twMerge( 'p-0 bg-surface overflow-hidden relative transition-height', // The first step's columns (w-56 + w-48 + w-80), held across steps. - // Without the output column when the output is preset (w-80 fewer). - singleKind - ? outputLocked - ? 'w-[12rem]' - : 'w-[32rem]' - : outputLocked - ? 'w-[26rem]' - : 'w-[46rem]', + singleKind ? 'w-[32rem]' : 'w-[46rem]', currentConfigStep === 'schedule' ? 'h-[27rem]' : currentConfigStep === 'asset' @@ -448,9 +426,7 @@ 'flex flex-col gap-1 p-2 overflow-auto transition-opacity w-48', selected.triggerId ? '' : 'opacity-20' )} - {@attach arrowTabNav({ - onKeyDown: selectAndAdvanceTo(() => (outputLocked ? pathEl : outputEl)) - })} + {@attach arrowTabNav({ onKeyDown: selectAndAdvanceTo(() => outputEl) })} >
Language
{#each languages as l} @@ -460,9 +436,6 @@ unifiedSize="sm" btnClasses="justify-start" selected={isSelected} - disabled={outputLocked && - !!selected.outputId && - !compatibleOutputKinds(l.lang).includes(selected.outputId)} onClick={() => { selected.language = l.lang const _compatibleOutputKinds = compatibleOutputKinds(l.lang) @@ -477,36 +450,34 @@ {/each} - {#if !outputLocked} -
pathEl, { timeout: 50 }) })} - > -
Output asset
- {#each visibleOutputKinds.length ? visibleOutputKinds : PIPELINE_OUTPUT_KINDS as k} - {@const isSelected = selected.outputId === k.id} - - {/each} -
- {/if} +
pathEl, { timeout: 50 }) })} + > +
Output asset
+ {#each visibleOutputKinds.length ? visibleOutputKinds : PIPELINE_OUTPUT_KINDS as k} + {@const isSelected = selected.outputId === k.id} + + {/each} +
{/snippet} {#snippet bottomSection(close: () => void)} diff --git a/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.test.ts b/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.test.ts index eec4b7a671..a2bfb3327d 100644 --- a/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.test.ts +++ b/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from 'vitest' import { + addTriggerDirective, listenToAsset, removeTriggerDirective, - scheduleInsteadOfAsset, stopListeningToAsset } from './pipelineAnnotationEdits' @@ -42,10 +42,9 @@ describe('pipeline annotation edits', () => { ) }) - it('swaps an asset subscription for a schedule, and drops a trigger kind', () => { - const ts = '// pipeline\n// on datatable://main/orders\nexport function main() {}' - expect(scheduleInsteadOfAsset(ts, table, false)).toBe( - '// pipeline\n// on schedule\nexport function main() {}' + it('adds a trigger kind once, and drops it', () => { + expect(addTriggerDirective('// pipeline\n// on webhook\nx()', 'webhook')).toBe( + '// pipeline\n// on webhook\nx()' ) expect(removeTriggerDirective('// pipeline\n// on kafka\nx()', 'kafka')).toBe( '// pipeline\nx()' diff --git a/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.ts b/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.ts index dc6d68b10f..d27ca73917 100644 --- a/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.ts +++ b/frontend/src/lib/components/assets/AssetGraph/pipelineAnnotationEdits.ts @@ -113,16 +113,6 @@ export function stopListeningToAsset( return out } -/** The script runs on a schedule rather than after writes to the asset. */ -export function scheduleInsteadOfAsset( - content: string, - asset: { kind: AssetKind; path: string }, - reads: boolean -): string { - const out = stopListeningToAsset(content, asset, reads) - return hasDirective(out, onTrigger('schedule')) ? out : addDirective(out, 'on schedule') -} - /** Adds a `// on ` line (`schedule`, `webhook`, …) unless it has one. */ export function addTriggerDirective(content: string, kind: string): string { return hasDirective(content, onTrigger(kind)) ? content : addDirective(content, `on ${kind}`) diff --git a/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte b/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte index e1fb91644d..e2e37cffc9 100644 --- a/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte @@ -105,7 +105,6 @@ assetRef, listenToAsset, removeTriggerDirective, - scheduleInsteadOfAsset, stopListeningChangeText, stopListeningToAsset } from '$lib/components/assets/AssetGraph/pipelineAnnotationEdits' @@ -1358,8 +1357,6 @@ } const scriptName = (p: string) => p.split('/').pop() ?? p - const isTableKind = (k: AssetKind): k is 'ducklake' | 'datatable' => - k === 'ducklake' || k === 'datatable' const readsAsset = (script: string, asset: { kind: AssetKind; path: string }) => shownGraph.edges.some( @@ -1385,77 +1382,9 @@ onClick: () => selectAsset(asset) }) - /** The fixes for an asset's unwritten subscription, from either end of it. */ - function subscriptionFixes( - asset: { kind: AssetKind; path: string }, - subscribers: string[] - ): NodeFixSpec['actions'] { - const one = subscribers.length === 1 ? subscribers[0] : undefined - const reads = subscribers.some((s) => readsAsset(s, asset)) - const actions: NodeFixSpec['actions'] = [] - if (isTableKind(asset.kind)) { - actions.push({ - label: 'Add a script that writes it', - detail: `Opens the new-script menu with ${assetRef(asset)} as its output.`, - addWriter: { kind: asset.kind, path: asset.path } - }) - } - if (one) { - actions.push({ - label: `Run ${scriptName(one)} on a schedule instead`, - detail: `Opens the schedule setup. On create: ${stopListeningChangeText(asset, reads).replace(/^R/, 'r')} and adds \`on schedule\` in ${one}, with the schedule as a draft.`, - scheduleFor: { - script: one, - onSchedule: async (schedule) => { - const ok = await editScriptHeader( - one, - (c) => scheduleInsteadOfAsset(c, asset, readsAsset(one, asset)), - 'Switched to a schedule' - ) - if (!ok) return - const saved = pe.setTriggerDraft({ - kind: 'schedule', - config: { ...schedule, script_path: one } - }) - if (!saved) { - sendUserToast( - `Another draft schedule already uses the path ${schedule.path}; ${one} has no schedule yet`, - true - ) - } - } - } - }) - } - actions.push({ - label: one ? `Stop ${scriptName(one)} running on writes` : 'Stop them running on writes', - detail: `${stopListeningChangeText(asset, reads)} in ${subscribers.join(', ')}; only the annotation header changes. Saved as a draft.`, - run: () => - editScripts( - subscribers, - 'Stopped running on writes', - (s) => (c) => stopListeningToAsset(c, asset, readsAsset(s, asset)) - ) - }) - return actions - } - let nodeFixes = $derived.by(() => { const fixes = new Map() if (isOperator) return fixes - for (const { asset, subscribers } of nodeErrors.unwritten.values()) { - fixes.set(`asset:${asset.kind}:${asset.path}`, { - explainer: [ - ...(subscribers.length === 1 - ? [scriptLink(subscribers[0]), ' runs'] - : [`${subscribers.length} scripts run`]), - ' after each write to ', - assetLink(asset), - `, but nothing writes it, so ${subscribers.length === 1 ? 'it never runs' : 'they never run'}.` - ], - actions: subscriptionFixes(asset, subscribers) - }) - } for (const [path, asset] of nodeErrors.waitsOn) { fixes.set(`script:${path}`, { explainer: [ @@ -1464,7 +1393,16 @@ ', but nothing writes it, so it never runs.' ], actions: [ - ...subscriptionFixes(asset, [path]), + { + label: `Remove \`on ${assetRef(asset)}\``, + detail: `${stopListeningChangeText(asset, readsAsset(path, asset))} in ${path}; only the annotation header changes. Saved as a draft.`, + run: () => + editScriptHeader( + path, + (c) => stopListeningToAsset(c, asset, readsAsset(path, asset)), + `Removed \`on ${assetRef(asset)}\`` + ) + }, { label: 'Open script', run: () => {