mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: keep the perpetual-run opt-in across relocks and check the new version's tag
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
722ff7820a
commit
8bbccb3300
+19
@@ -0,0 +1,19 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at, apply_to_perpetual_runs)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp(), apply_to_perpetual_runs\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Int8",
|
||||
"Text",
|
||||
"Text",
|
||||
"Jsonb",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "6fb0cdaffcbda46868d5049057bc2898a523acb7e4ac13eb803c12553de34b62"
|
||||
}
|
||||
-19
@@ -1,19 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp()\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Int8",
|
||||
"Text",
|
||||
"Text",
|
||||
"Jsonb",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT on_behalf_of IS NOT NULL AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2",
|
||||
"query": "SELECT (on_behalf_of IS NOT NULL OR on_behalf_of_email IS NOT NULL) AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -19,5 +19,5 @@
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "cf14e4d52fb9d35141a76e6d38973fb2000ad85d1a4e065c6ef4305a4b4e800e"
|
||||
"hash": "fa5009b8f8582408969c47db184f2d5cb52ab4db7023f10ea4269c3ae4176a6d"
|
||||
}
|
||||
@@ -4,6 +4,7 @@
|
||||
use serde_json::{json, Value};
|
||||
use sqlx::{types::Json, Pool, Postgres};
|
||||
use uuid::Uuid;
|
||||
use windmill_common::scripts::{deploy_relocked_version, fetch_script_for_update};
|
||||
use windmill_queue::{add_completed_job, get_mini_completed_job};
|
||||
|
||||
const W_ID: &str = "test-workspace";
|
||||
@@ -113,6 +114,26 @@ async fn a_flagged_newer_version_takes_over_the_next_run(db: Pool<Postgres>) ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A dependency change relocks a version into a new one, which has to keep the flag or the loop
|
||||
/// never moves.
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn a_flagged_version_relocked_before_the_run_ends_still_takes_over(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
insert_version(&db, 4001, 60.0, None, None, None).await?;
|
||||
insert_version(&db, 4002, 0.0, None, Some(true), None).await?;
|
||||
let mut tx = db.begin().await?;
|
||||
let head = fetch_script_for_update(PATH, W_ID, &mut *tx)
|
||||
.await?
|
||||
.unwrap();
|
||||
let relocked = deploy_relocked_version(&mut tx, head, None, Some(""), None, None).await?;
|
||||
tx.commit().await?;
|
||||
|
||||
let (hash, _, _) = next_run_after_run_of(&db, 4001).await?;
|
||||
assert_eq!(hash, relocked);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn a_newer_version_without_the_flag_leaves_the_run_on_its_version(
|
||||
db: Pool<Postgres>,
|
||||
|
||||
@@ -648,8 +648,8 @@ struct CreateScriptQuery {
|
||||
#[serde(default)]
|
||||
skip_if_noop: bool,
|
||||
/// Perpetual runs of an older version at this path start their next run on the deployed
|
||||
/// version once their current one finishes. A deploy skipped as a no-op creates no version,
|
||||
/// so it moves nothing.
|
||||
/// version once their current one finishes, or stop when it is not perpetual. A deploy skipped
|
||||
/// as a no-op creates no version, so it moves nothing.
|
||||
#[serde(default)]
|
||||
apply_to_perpetual_runs: bool,
|
||||
}
|
||||
|
||||
@@ -10200,7 +10200,8 @@ paths:
|
||||
description: |
|
||||
Move perpetual runs of an older version at this path to the deployed version. Each run
|
||||
finishes on its own version, and the next one starts on this version with its settings
|
||||
and the finished run's arguments.
|
||||
and the finished run's arguments. If this version is not perpetual, the runs stop
|
||||
after their current run instead.
|
||||
in: query
|
||||
schema:
|
||||
type: boolean
|
||||
|
||||
@@ -465,7 +465,7 @@ pub async fn deploy_relocked_version(
|
||||
dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \
|
||||
delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \
|
||||
codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, \
|
||||
lock_error_logs, created_at)
|
||||
lock_error_logs, created_at, apply_to_perpetual_runs)
|
||||
|
||||
SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, \
|
||||
content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, \
|
||||
@@ -473,7 +473,7 @@ pub async fn deploy_relocked_version(
|
||||
dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \
|
||||
delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \
|
||||
codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, \
|
||||
$6::text, clock_timestamp()
|
||||
$6::text, clock_timestamp(), apply_to_perpetual_runs
|
||||
|
||||
FROM script WHERE hash = $2 AND workspace_id = $3;
|
||||
", new_hash, s.hash.0, w_id, lock, modules_json, lock_error_logs).execute(&mut **tx).await?;
|
||||
|
||||
@@ -1788,10 +1788,11 @@ async fn perpetual_version_switch(
|
||||
None => {
|
||||
// A run of a version with an on-behalf-of identity is permissioned as that identity,
|
||||
// not as whoever started the loop. Carrying it over would run a version that no
|
||||
// longer names it with its authority.
|
||||
// longer names it with its authority. An address-only row counts: a server predating
|
||||
// the principal column dispatched it as that address.
|
||||
let ran_on_behalf_of = sqlx::query_scalar!(
|
||||
"SELECT on_behalf_of IS NOT NULL AS \"ran_on_behalf_of!\" FROM script \
|
||||
WHERE hash = $1 AND workspace_id = $2",
|
||||
"SELECT (on_behalf_of IS NOT NULL OR on_behalf_of_email IS NOT NULL) \
|
||||
AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2",
|
||||
hash.0,
|
||||
w_id
|
||||
)
|
||||
@@ -1811,6 +1812,27 @@ async fn perpetual_version_switch(
|
||||
)
|
||||
}
|
||||
};
|
||||
// The loop's tag was checked when it started; a new version's own tag has not been.
|
||||
if latest.dedicated_worker != Some(true) {
|
||||
if let Some(tag) = latest.tag.as_deref().filter(|t| !t.is_empty()) {
|
||||
let is_super_admin = windmill_common::auth::is_super_admin_email(db, &email).await?;
|
||||
if let Err(e) = windmill_common::jobs::check_tag_available_for_workspace_internal(
|
||||
db,
|
||||
w_id,
|
||||
tag,
|
||||
is_super_admin,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(
|
||||
"Perpetual script {path} stays on version {hash}: version {} has tag {tag}: {e}",
|
||||
ScriptHash(latest.hash)
|
||||
);
|
||||
return Ok(PerpetualVersionSwitch::Stay);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(PerpetualVersionSwitch::To(PerpetualNextRun {
|
||||
hash: ScriptHash(latest.hash),
|
||||
language: latest.language,
|
||||
|
||||
@@ -706,6 +706,13 @@ export async function handleFile(
|
||||
deepEqual(modules ?? null, remote.modules ?? null))
|
||||
) {
|
||||
log.info(colors.green(`Script ${remotePath} is up to date`));
|
||||
if (opts?.applyToPerpetualRuns) {
|
||||
log.warn(
|
||||
colors.yellow(
|
||||
`No new version of ${remotePath} was deployed, so --apply-to-perpetual-runs moves no runs`
|
||||
)
|
||||
);
|
||||
}
|
||||
// Even when the body is unchanged, perms may still drift — sync them
|
||||
// independently before returning.
|
||||
await applyExtraPermsDiff(
|
||||
@@ -2226,7 +2233,7 @@ const command = new Command()
|
||||
.option("--message <message:string>", "Deployment message")
|
||||
.option(
|
||||
"--apply-to-perpetual-runs",
|
||||
"Move running perpetual runs of this script to the new version once their current run finishes",
|
||||
"Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual",
|
||||
)
|
||||
.action(push as any)
|
||||
.command("get", "get a script's details")
|
||||
|
||||
@@ -6873,7 +6873,7 @@ const command = new Command()
|
||||
)
|
||||
.option(
|
||||
"--apply-to-perpetual-runs",
|
||||
"Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes",
|
||||
"Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual",
|
||||
)
|
||||
.action(push as any)
|
||||
// Internal: invoked only by the git-sync hub script. Hidden from help and
|
||||
|
||||
Generated
+2
-2
@@ -7697,7 +7697,7 @@ script related commands
|
||||
- \`--json\` - Output as JSON (for piping to jq)
|
||||
- \`script push <path:file>\` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh)
|
||||
- \`--message <message:string>\` - Deployment message
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
- \`script get <path:file>\` - get a script's details
|
||||
- \`--json\` - Output as JSON (for piping to jq)
|
||||
- \`script show <path:file>\` - show a script's content (alias for get)
|
||||
@@ -7793,7 +7793,7 @@ sync local with a remote workspaces or the opposite (push or pull)
|
||||
- \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks
|
||||
- \`--auto-metadata\` - Automatically regenerate stale metadata (locks and schemas) before pushing
|
||||
- \`--accept-overriding-permissioned-as-with-self\` - Accept that items with a different permissioned_as will be updated with your own user
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
|
||||
### token
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ export type PerpetualRunsAtPath = {
|
||||
count: number
|
||||
/** More runs than one page holds are queued at the path, so `count` is a lower bound. */
|
||||
truncated: boolean
|
||||
/** Arguments the deployed schema adds as required, removes, or retypes, compared with the runs' versions. */
|
||||
/** Arguments the deployed schema removes, retypes or newly requires, compared with the runs' versions. */
|
||||
mismatchedArgs: string[]
|
||||
}
|
||||
|
||||
@@ -22,23 +22,35 @@ export async function loadPerpetualRunsAtPath(
|
||||
jobKinds: 'script',
|
||||
perPage: RUNS_PAGE_SIZE
|
||||
})
|
||||
// A flow step running this script never restarts, whatever the script's perpetual setting.
|
||||
const runs = queued.filter((job) => !job.parent_job)
|
||||
// Only what the backend restarts: never a flow step, and only a run of a perpetual version.
|
||||
const candidates = queued.filter((job) => !job.is_flow_step && job.script_hash)
|
||||
const hashes = [...new Set(candidates.map((job) => job.script_hash!))]
|
||||
const versions = new Map(
|
||||
await Promise.all(
|
||||
hashes.map(
|
||||
async (hash) =>
|
||||
[
|
||||
hash,
|
||||
await ScriptService.getScriptByHash({ workspace, hash }).catch(() => undefined)
|
||||
] as const
|
||||
)
|
||||
)
|
||||
)
|
||||
const runs = candidates.filter((job) => versions.get(job.script_hash!)?.restart_unless_cancelled)
|
||||
if (runs.length === 0) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const hashes = [...new Set(runs.flatMap((job) => (job.script_hash ? [job.script_hash] : [])))]
|
||||
const versions = await Promise.all(
|
||||
hashes.map((hash) => ScriptService.getScriptByHash({ workspace, hash }).catch(() => undefined))
|
||||
)
|
||||
const mismatchedArgs = new Set<string>()
|
||||
for (const version of versions) {
|
||||
if (!version) continue
|
||||
for (const [arg, { diff }] of Object.entries(computeDiff(schema, version.schema))) {
|
||||
// A new optional argument takes its default when the reused arguments lack it.
|
||||
if (diff === 'same' || (diff === 'added' && !schema?.required?.includes(arg))) continue
|
||||
mismatchedArgs.add(arg)
|
||||
for (const hash of new Set(runs.map((job) => job.script_hash!))) {
|
||||
const previous = versions.get(hash)?.schema
|
||||
for (const [arg, { diff }] of Object.entries(computeDiff(schema, previous))) {
|
||||
// An added argument only breaks a reused run when it is required, checked below.
|
||||
if (diff !== 'same' && diff !== 'added') mismatchedArgs.add(arg)
|
||||
}
|
||||
const previouslyRequired: unknown[] = Array.isArray(previous?.required) ? previous.required : []
|
||||
for (const arg of schema?.required ?? []) {
|
||||
if (!previouslyRequired.includes(arg)) mismatchedArgs.add(arg)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -556,7 +556,7 @@ script related commands
|
||||
- `--json` - Output as JSON (for piping to jq)
|
||||
- `script push <path:file>` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh)
|
||||
- `--message <message:string>` - Deployment message
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
- `script get <path:file>` - get a script's details
|
||||
- `--json` - Output as JSON (for piping to jq)
|
||||
- `script show <path:file>` - show a script's content (alias for get)
|
||||
@@ -652,7 +652,7 @@ sync local with a remote workspaces or the opposite (push or pull)
|
||||
- `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks
|
||||
- `--auto-metadata` - Automatically regenerate stale metadata (locks and schemas) before pushing
|
||||
- `--accept-overriding-permissioned-as-with-self` - Accept that items with a different permissioned_as will be updated with your own user
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
|
||||
### token
|
||||
|
||||
|
||||
@@ -3841,7 +3841,7 @@ script related commands
|
||||
- \`--json\` - Output as JSON (for piping to jq)
|
||||
- \`script push <path:file>\` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh)
|
||||
- \`--message <message:string>\` - Deployment message
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
- \`script get <path:file>\` - get a script's details
|
||||
- \`--json\` - Output as JSON (for piping to jq)
|
||||
- \`script show <path:file>\` - show a script's content (alias for get)
|
||||
@@ -3937,7 +3937,7 @@ sync local with a remote workspaces or the opposite (push or pull)
|
||||
- \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks
|
||||
- \`--auto-metadata\` - Automatically regenerate stale metadata (locks and schemas) before pushing
|
||||
- \`--accept-overriding-permissioned-as-with-self\` - Accept that items with a different permissioned_as will be updated with your own user
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes
|
||||
- \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
|
||||
### token
|
||||
|
||||
|
||||
@@ -561,7 +561,7 @@ script related commands
|
||||
- `--json` - Output as JSON (for piping to jq)
|
||||
- `script push <path:file>` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh)
|
||||
- `--message <message:string>` - Deployment message
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
- `script get <path:file>` - get a script's details
|
||||
- `--json` - Output as JSON (for piping to jq)
|
||||
- `script show <path:file>` - show a script's content (alias for get)
|
||||
@@ -657,7 +657,7 @@ sync local with a remote workspaces or the opposite (push or pull)
|
||||
- `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks
|
||||
- `--auto-metadata` - Automatically regenerate stale metadata (locks and schemas) before pushing
|
||||
- `--accept-overriding-permissioned-as-with-self` - Accept that items with a different permissioned_as will be updated with your own user
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes
|
||||
- `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual
|
||||
|
||||
### token
|
||||
|
||||
|
||||
Reference in New Issue
Block a user