mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-12 16:05:43 +00:00
fix: resolve a script path to its new version as soon as the lock lands (#10794)
* fix: resolve a script path to its new version as soon as the lock lands Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5ygpzQHkPeYsjiP9GzBy * fix: tell MCP script deploy callers to stop polling on a lock error Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5ygpzQHkPeYsjiP9GzBy --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
449b1a6933
commit
3c8e4b43fd
+30
@@ -0,0 +1,30 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT\n (SELECT hash FROM script\n WHERE path = $1 AND workspace_id = $2 AND deleted = false\n AND lock IS NOT NULL AND lock_error_logs IS NULL\n ORDER BY created_at DESC LIMIT 1) AS hash,\n (SELECT lock IS NULL AND lock_error_logs IS NULL\n AND created_at > now() - make_interval(secs => $3) FROM script\n WHERE path = $1 AND workspace_id = $2 AND deleted = false\n ORDER BY created_at DESC LIMIT 1) AS pending_lock",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "hash",
|
||||
"type_info": "Int8"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "pending_lock",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Float8"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null,
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "bf4e451d2748c880ce5bd189037dda592a71717644c9c4206721de16cff59392"
|
||||
}
|
||||
@@ -0,0 +1,7 @@
|
||||
DROP TRIGGER IF EXISTS script_insert_trigger ON script;
|
||||
|
||||
CREATE TRIGGER script_insert_trigger
|
||||
AFTER INSERT ON script
|
||||
FOR EACH ROW
|
||||
WHEN (NEW.lock IS NOT NULL)
|
||||
EXECUTE FUNCTION notify_runnable_version_change('script');
|
||||
@@ -0,0 +1,10 @@
|
||||
-- A version deployed without a lockfile gets one later from a dependency job, so gating the
|
||||
-- insert notification on `lock IS NOT NULL` left the path-to-hash caches holding the previous
|
||||
-- version until that UPDATE (or their TTL) came around. Notify on every insert instead: the
|
||||
-- deploy itself is what makes the cached answer suspect.
|
||||
DROP TRIGGER IF EXISTS script_insert_trigger ON script;
|
||||
|
||||
CREATE TRIGGER script_insert_trigger
|
||||
AFTER INSERT ON script
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION notify_runnable_version_change('script');
|
||||
+8
-5
@@ -1762,11 +1762,14 @@ async fn process_notify_event(
|
||||
let key = (workspace_id.to_string(), path.to_string());
|
||||
match *source_type {
|
||||
"script" => {
|
||||
windmill_common::DEPLOYED_SCRIPT_HASH_CACHE.remove(&key);
|
||||
// Bundle-cache key resolution for imported scripts; evicted
|
||||
// together with the content-side caches below so key and
|
||||
// inlined content flip to the new version in the same window.
|
||||
windmill_common::IMPORTED_SCRIPT_HASH_CACHE.remove(&key);
|
||||
// Evicts DEPLOYED_SCRIPT_HASH_CACHE together with the bundle-cache
|
||||
// key resolution for imported scripts (IMPORTED_SCRIPT_HASH_CACHE),
|
||||
// so key and inlined content flip to the new version in the same
|
||||
// window as the content-side cache below.
|
||||
windmill_common::invalidate_latest_script_hash_caches(
|
||||
workspace_id,
|
||||
path,
|
||||
);
|
||||
// Evict the relative-import latest-hash cache so a redeployed
|
||||
// imported script flips the content cache to its new version
|
||||
// across all replicas within a poll interval (see #6769). Keyed
|
||||
|
||||
@@ -547,6 +547,7 @@ async fn create_snapshot_script(
|
||||
let mut uploaded = false;
|
||||
let mut handle_deployment_metadata = None;
|
||||
let mut moved_native_triggers = Vec::new();
|
||||
let mut deployed_path = None;
|
||||
while let Some(field) = multipart.next_field().await.unwrap() {
|
||||
let name = field.name().unwrap().to_string();
|
||||
let data = field.bytes().await.unwrap();
|
||||
@@ -554,6 +555,7 @@ async fn create_snapshot_script(
|
||||
let ns: NewScript = Some(serde_json::from_slice(&data).map_err(to_anyhow)?).unwrap();
|
||||
let is_tar = ns.codebase.as_ref().is_some_and(|x| x.ends_with(".tar"));
|
||||
let use_esm = ns.codebase.as_ref().is_some_and(|x| x.contains(".esm"));
|
||||
deployed_path = Some(ns.path.clone());
|
||||
let (new_hash, ntx, hdm, moved) = create_script_internal(
|
||||
ns,
|
||||
w_id.clone(),
|
||||
@@ -606,6 +608,9 @@ async fn create_snapshot_script(
|
||||
}
|
||||
|
||||
tx.unwrap().commit().await?;
|
||||
if let Some(script_path) = deployed_path.as_deref() {
|
||||
invalidate_script_path_caches(&w_id, script_path);
|
||||
}
|
||||
reregister_moved_native_triggers(&db, &authed, &w_id, moved_native_triggers);
|
||||
if let Some(hdm) = handle_deployment_metadata {
|
||||
hdm.handle(&db).await?;
|
||||
@@ -741,6 +746,7 @@ async fn deploy_script(
|
||||
)
|
||||
.await?;
|
||||
tx.commit().await?;
|
||||
invalidate_script_path_caches(&w_id, &script_path);
|
||||
reregister_moved_native_triggers(&db, &authed_for_triggers, &w_id, moved_native_triggers);
|
||||
if let Some(hdm) = hdm {
|
||||
// Only a script that needed no lock generation is deployed and runnable by
|
||||
@@ -770,6 +776,14 @@ async fn deploy_script(
|
||||
Ok((StatusCode::CREATED, format!("{}", hash)))
|
||||
}
|
||||
|
||||
/// Drop the path -> hash entries a just-committed deploy made stale, so this process resolves the
|
||||
/// path to the new version straight away instead of at the next `notify_event` poll. Must run
|
||||
/// after the commit, or a concurrent resolution repopulates them from the pre-deploy state.
|
||||
fn invalidate_script_path_caches(w_id: &str, script_path: &str) {
|
||||
windmill_common::invalidate_latest_script_hash_caches(w_id, script_path);
|
||||
RAW_SCRIPT_LATEST_HASH_CACHE.remove(&format!("{w_id}:{script_path}"));
|
||||
}
|
||||
|
||||
/// What a script deploy still has to do once its transaction has committed.
|
||||
enum PostCommitDeploy {
|
||||
/// Everything, for a deploy with no dependency job to hand it to.
|
||||
|
||||
@@ -12771,7 +12771,7 @@
|
||||
"description": "Creates a new script at a path that does not already hold one.\nSupplying `parent_hash` instead deploys a new version of the script that hash names, and a `path` differing from that version's moves the script there. `POST /w/{workspace}/scripts/update/{path}` does the same, naming the superseded version in its URL.\n",
|
||||
"operationId": "createScript",
|
||||
"x-mcp-tool": true,
|
||||
"x-mcp-instructions": "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one.",
|
||||
"x-mcp-instructions": "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on.",
|
||||
"x-mcp-tool-include-fields": [
|
||||
"path",
|
||||
"content",
|
||||
@@ -12822,7 +12822,7 @@
|
||||
"description": "Deploys a new version of the script at `path`, which must already hold one.\nThe body's `path` is the destination: the same path leaves the script where it\nis, a different one moves it there and archives the old path.\n",
|
||||
"operationId": "updateScript",
|
||||
"x-mcp-tool": true,
|
||||
"x-mcp-instructions": "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one.",
|
||||
"x-mcp-instructions": "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on.",
|
||||
"x-mcp-tool-include-fields": [
|
||||
"path",
|
||||
"content",
|
||||
|
||||
@@ -13315,8 +13315,12 @@ paths:
|
||||
For TypeScript, use 'bun' unless deno-specific APIs are needed. A path
|
||||
that already holds a script is refused: use updateScript to deploy a
|
||||
new version of it, and do NOT delete and recreate a script to change
|
||||
it. A new version generates its lock async and can take up to a minute
|
||||
before a run by path uses it rather than the previous one.
|
||||
it. A new version generates its lock async, and only a version with a
|
||||
lock is runnable: until it lands, a run by path still executes the
|
||||
previous version. Poll getScriptByPath before running the new one and
|
||||
stop on either outcome: lock non-null means it is ready, lock_error_logs
|
||||
set means the lockfile failed and that version will never run, so report
|
||||
the error instead of polling on.
|
||||
x-mcp-tool-include-fields:
|
||||
- path
|
||||
- content
|
||||
@@ -13519,8 +13523,12 @@ paths:
|
||||
unless you wrote its content yourself. Set path__body only to move the
|
||||
script to a different path; omit it to leave the script where it is. A
|
||||
path that holds no script is refused: use createScript to create one. A
|
||||
new version generates its lock async and can take up to a minute before
|
||||
a run by path uses it rather than the previous one.
|
||||
new version generates its lock async, and only a version with a lock is
|
||||
runnable: until it lands, a run by path still executes the previous
|
||||
version. Poll getScriptByPath before running the new one and stop on
|
||||
either outcome: lock non-null means it is ready, lock_error_logs set
|
||||
means the lockfile failed and that version will never run, so report the
|
||||
error instead of polling on.
|
||||
x-mcp-tool-include-fields:
|
||||
- path
|
||||
- content
|
||||
|
||||
@@ -9454,7 +9454,7 @@ paths:
|
||||
x-mcp-tool: true
|
||||
# The description above names `parent_hash`, which this tool does not expose.
|
||||
x-mcp-tool-description: Creates a script at a path that does not already hold one.
|
||||
x-mcp-instructions: "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one."
|
||||
x-mcp-instructions: "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on."
|
||||
x-mcp-tool-include-fields:
|
||||
- path
|
||||
- content
|
||||
@@ -9493,7 +9493,7 @@ paths:
|
||||
is, a different one moves it there and archives the old path.
|
||||
operationId: updateScript
|
||||
x-mcp-tool: true
|
||||
x-mcp-instructions: "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one."
|
||||
x-mcp-instructions: "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on."
|
||||
x-mcp-tool-include-fields:
|
||||
- path
|
||||
- content
|
||||
|
||||
@@ -679,7 +679,7 @@ pub fn all_tools() -> Vec<EndpointTool> {
|
||||
EndpointTool {
|
||||
name: Cow::Borrowed("createScript"),
|
||||
description: Cow::Borrowed("create script: Creates a script at a path that does not already hold one"),
|
||||
instructions: Cow::Borrowed("Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one."),
|
||||
instructions: Cow::Borrowed("Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on."),
|
||||
path: Cow::Borrowed("/w/{workspace}/scripts/create"),
|
||||
method: Cow::Borrowed("POST"),
|
||||
path_params_schema: None,
|
||||
@@ -730,7 +730,7 @@ pub fn all_tools() -> Vec<EndpointTool> {
|
||||
description: Cow::Borrowed("update script: Deploys a new version of the script at `path`, which must already hold one.
|
||||
The body's `path` is the destination: the same path leaves the script where it
|
||||
is, a different one moves it there and archives the old path"),
|
||||
instructions: Cow::Borrowed("Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one."),
|
||||
instructions: Cow::Borrowed("Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on."),
|
||||
path: Cow::Borrowed("/w/{workspace}/scripts/update/{path}"),
|
||||
method: Cow::Borrowed("POST"),
|
||||
path_params_schema: Some(serde_json::json!({
|
||||
|
||||
@@ -404,6 +404,19 @@ lazy_static::lazy_static! {
|
||||
|
||||
const LATEST_VERSION_ID_CACHE_TTL: std::time::Duration = std::time::Duration::from_secs(60);
|
||||
|
||||
/// TTL for a path -> hash answer that a dependency job is about to invalidate by writing the
|
||||
/// lockfile of a newer version. That job lands at an unpredictable moment and the eviction it
|
||||
/// notifies only reaches this process on the next `notify_event` poll, so the entry must not
|
||||
/// outlive it by more than a beat.
|
||||
const LATEST_VERSION_ID_PENDING_LOCK_CACHE_TTL: std::time::Duration =
|
||||
std::time::Duration::from_secs(2);
|
||||
|
||||
/// How long a version without a lockfile is still believed to have a dependency job coming for
|
||||
/// it. A job that is cancelled while queued, or whose worker dies before it can write
|
||||
/// `lock_error_logs`, leaves that version pending for good; past this age the short TTL above
|
||||
/// would be a permanent cost for a version that is never going to become runnable.
|
||||
const PENDING_LOCK_MAX_AGE: std::time::Duration = std::time::Duration::from_secs(10 * 60);
|
||||
|
||||
/// Test hook: disables the process-global deployed-script hash/info caches so
|
||||
/// every resolution reads the current DB. Integration tests use `#[sqlx::test]`
|
||||
/// isolated DBs that share one workspace id and reuse script paths, so a cache
|
||||
@@ -1843,11 +1856,12 @@ pub fn get_latest_deployed_hash_for_path<'e>(
|
||||
}
|
||||
_ => {
|
||||
tracing::debug!("Fetching script hash for {script_path}");
|
||||
let hash = if let Some(db) = db {
|
||||
let latest = if let Some(db) = db {
|
||||
let authed = db.authed;
|
||||
let mut conn = db.acquire().await?;
|
||||
let hash = get_latest_script_hash(&mut *conn, script_path, w_id).await?;
|
||||
if let Some(hash) = hash {
|
||||
let latest =
|
||||
get_latest_deployed_script_hash(&mut *conn, script_path, w_id).await?;
|
||||
if let Some(hash) = latest.hash {
|
||||
HASH_PERMS_CACHE.insert(
|
||||
computed_hash.unwrap_or_else(|| PermsCache::compute_hash(authed)),
|
||||
ScriptHash(hash),
|
||||
@@ -1861,19 +1875,24 @@ pub fn get_latest_deployed_hash_for_path<'e>(
|
||||
return Err(Error::NotAuthorized(format!("You are not authorized to access this script: {script_path} (but it exists). Your permissions are: {:?}", authed)));
|
||||
}
|
||||
}
|
||||
hash
|
||||
latest
|
||||
} else {
|
||||
let mut conn = db2.acquire().await?;
|
||||
get_latest_script_hash(&mut *conn, script_path, w_id).await?
|
||||
get_latest_deployed_script_hash(&mut *conn, script_path, w_id).await?
|
||||
};
|
||||
|
||||
let hash = utils::not_found_if_none(hash, "script", script_path)?;
|
||||
let hash = utils::not_found_if_none(latest.hash, "script", script_path)?;
|
||||
if use_cache {
|
||||
let ttl = if latest.pending_lock {
|
||||
LATEST_VERSION_ID_PENDING_LOCK_CACHE_TTL
|
||||
} else {
|
||||
LATEST_VERSION_ID_CACHE_TTL
|
||||
};
|
||||
DEPLOYED_SCRIPT_HASH_CACHE.insert(
|
||||
cache_key,
|
||||
ExpiringLatestVersionId {
|
||||
id: hash,
|
||||
expires_at: std::time::Instant::now() + LATEST_VERSION_ID_CACHE_TTL,
|
||||
expires_at: std::time::Instant::now() + ttl,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -1901,6 +1920,60 @@ pub async fn get_latest_script_hash<'e, E: sqlx::PgExecutor<'e>>(
|
||||
return Ok(hash);
|
||||
}
|
||||
|
||||
pub struct LatestDeployedScriptHash {
|
||||
pub hash: Option<i64>,
|
||||
/// The newest version of the path is still waiting on the dependency job that writes its
|
||||
/// lockfile, so `hash` points at the version before it and will change the moment that job
|
||||
/// lands, at a moment nothing notifies the caller of.
|
||||
pub pending_lock: bool,
|
||||
}
|
||||
|
||||
/// Applies no authorization of its own, exactly like [`get_latest_script_hash`]: pass an
|
||||
/// RLS-scoped executor, or check the caller's permissions on the hash it returns.
|
||||
pub async fn get_latest_deployed_script_hash<'e, E: sqlx::PgExecutor<'e>>(
|
||||
db: E,
|
||||
script_path: &'e str,
|
||||
w_id: &'e str,
|
||||
) -> error::Result<LatestDeployedScriptHash> {
|
||||
let row = sqlx::query!(
|
||||
"SELECT
|
||||
(SELECT hash FROM script
|
||||
WHERE path = $1 AND workspace_id = $2 AND deleted = false
|
||||
AND lock IS NOT NULL AND lock_error_logs IS NULL
|
||||
ORDER BY created_at DESC LIMIT 1) AS hash,
|
||||
(SELECT lock IS NULL AND lock_error_logs IS NULL
|
||||
AND created_at > now() - make_interval(secs => $3) FROM script
|
||||
WHERE path = $1 AND workspace_id = $2 AND deleted = false
|
||||
ORDER BY created_at DESC LIMIT 1) AS pending_lock",
|
||||
script_path,
|
||||
w_id,
|
||||
PENDING_LOCK_MAX_AGE.as_secs_f64()
|
||||
)
|
||||
.fetch_one(db)
|
||||
.await?;
|
||||
Ok(
|
||||
LatestDeployedScriptHash {
|
||||
hash: row.hash,
|
||||
pending_lock: row.pending_lock.unwrap_or(false),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Drop this process's path -> runnable-hash entry for a script whose newest runnable version
|
||||
/// just moved, so the process that deployed it (or that generated its lockfile) resolves the
|
||||
/// path to it without waiting out the `notify_event` poll. Other replicas get there through
|
||||
/// `notify_runnable_version_change`.
|
||||
pub fn invalidate_deployed_script_hash_cache(w_id: &str, script_path: &str) {
|
||||
DEPLOYED_SCRIPT_HASH_CACHE.remove(&(w_id.to_string(), script_path.to_string()));
|
||||
}
|
||||
|
||||
/// Same, for a new version row, which also moves the import-side answer (that one has no lock
|
||||
/// predicate, so only a new row moves it).
|
||||
pub fn invalidate_latest_script_hash_caches(w_id: &str, script_path: &str) {
|
||||
invalidate_deployed_script_hash_cache(w_id, script_path);
|
||||
IMPORTED_SCRIPT_HASH_CACHE.remove(&(w_id.to_string(), script_path.to_string()));
|
||||
}
|
||||
|
||||
/// Latest non-archived hash for an imported `path`, for bundle cache keying.
|
||||
/// MUST select the same row as the bundler's content endpoint
|
||||
/// (`raw_script_by_path_internal`: `archived = false ORDER BY created_at DESC`,
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::get_latest_deployed_script_hash;
|
||||
|
||||
const WORKSPACE: &str = "test-workspace";
|
||||
|
||||
async fn deploy_version(db: &Pool<Postgres>, path: &str, hash: i64, lock: Option<&str>, age: f64) {
|
||||
sqlx::query(
|
||||
"INSERT INTO script (workspace_id, hash, path, summary, description, content, created_by, \
|
||||
language, kind, lock, created_at) \
|
||||
VALUES ($1, $2, $3, '', '', 'def main(): pass', 'test-user', 'python3', 'script', $4, \
|
||||
now() - make_interval(secs => $5))",
|
||||
)
|
||||
.bind(WORKSPACE)
|
||||
.bind(hash)
|
||||
.bind(path)
|
||||
.bind(lock)
|
||||
.bind(age)
|
||||
.execute(db)
|
||||
.await
|
||||
.expect("failed to deploy script version");
|
||||
}
|
||||
|
||||
/// A version whose dependency job has not written its lockfile yet is not runnable, so the answer
|
||||
/// stays on the version before it. Callers cache that answer, and only `pending_lock` tells them
|
||||
/// the dependency job is about to change it under them.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn pending_lock_flags_an_answer_a_dependency_job_is_about_to_change(db: Pool<Postgres>) {
|
||||
let path = "f/test/probe";
|
||||
deploy_version(&db, path, 1, Some(""), 2.0).await;
|
||||
|
||||
let latest = get_latest_deployed_script_hash(&db, path, WORKSPACE)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(latest.hash, Some(1));
|
||||
assert!(!latest.pending_lock);
|
||||
|
||||
deploy_version(&db, path, 2, None, 1.0).await;
|
||||
|
||||
let latest = get_latest_deployed_script_hash(&db, path, WORKSPACE)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(latest.hash, Some(1), "an unlocked version is not runnable");
|
||||
assert!(latest.pending_lock, "but it is about to become the answer");
|
||||
|
||||
sqlx::query("UPDATE script SET lock = '' WHERE workspace_id = $1 AND hash = 2")
|
||||
.bind(WORKSPACE)
|
||||
.execute(&db)
|
||||
.await
|
||||
.expect("failed to write the lockfile");
|
||||
|
||||
let latest = get_latest_deployed_script_hash(&db, path, WORKSPACE)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(latest.hash, Some(2));
|
||||
assert!(!latest.pending_lock);
|
||||
}
|
||||
|
||||
/// A version that will never become runnable must not keep the path on the short TTL: its
|
||||
/// dependency job either reported a failure, or is old enough that it is not coming.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn pending_lock_gives_up_on_a_dependency_job_that_will_not_land(db: Pool<Postgres>) {
|
||||
let failed = "f/test/failed";
|
||||
deploy_version(&db, failed, 1, Some(""), 2.0).await;
|
||||
deploy_version(&db, failed, 2, None, 1.0).await;
|
||||
sqlx::query("UPDATE script SET lock_error_logs = 'boom' WHERE workspace_id = $1 AND hash = 2")
|
||||
.bind(WORKSPACE)
|
||||
.execute(&db)
|
||||
.await
|
||||
.expect("failed to report the lock error");
|
||||
|
||||
let latest = get_latest_deployed_script_hash(&db, failed, WORKSPACE)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(latest.hash, Some(1));
|
||||
assert!(!latest.pending_lock);
|
||||
|
||||
let abandoned = "f/test/abandoned";
|
||||
deploy_version(&db, abandoned, 3, Some(""), 3600.0).await;
|
||||
deploy_version(&db, abandoned, 4, None, 1800.0).await;
|
||||
|
||||
let latest = get_latest_deployed_script_hash(&db, abandoned, WORKSPACE)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(latest.hash, Some(3));
|
||||
assert!(!latest.pending_lock);
|
||||
}
|
||||
@@ -492,6 +492,8 @@ async fn test_trigger_notify_runnable_version_change_script(db: Pool<Postgres>)
|
||||
let script_path = format!("f/test/script_{}", uuid::Uuid::new_v4());
|
||||
let script_hash: i64 = rand::random::<i64>().abs();
|
||||
|
||||
let before_insert_id = get_latest_event_id(&db).await.unwrap();
|
||||
|
||||
sqlx::query(
|
||||
"INSERT INTO script (workspace_id, hash, path, summary, description, content, created_by, language, kind)
|
||||
VALUES ('test-workspace', $1, $2, 'test', 'test', 'def main(): pass', 'test-user', 'python3', 'script')",
|
||||
@@ -502,6 +504,20 @@ async fn test_trigger_notify_runnable_version_change_script(db: Pool<Postgres>)
|
||||
.await
|
||||
.expect("Failed to insert script");
|
||||
|
||||
// The deploy has to notify even though the version is not runnable yet: it is what makes the
|
||||
// path -> hash caches suspect, and waiting for the lock UPDATE leaves them serving the
|
||||
// previous version for a poll interval past the moment the new one becomes runnable.
|
||||
let insert_events = poll_notify_events(&db, before_insert_id)
|
||||
.await
|
||||
.expect("Should poll events");
|
||||
assert!(
|
||||
insert_events
|
||||
.iter()
|
||||
.any(|e| e.channel == "notify_runnable_version_change"
|
||||
&& e.payload.starts_with("test-workspace:script:")),
|
||||
"Should have notify_runnable_version_change event for a version deployed without a lock"
|
||||
);
|
||||
|
||||
let before_id = get_latest_event_id(&db).await.unwrap();
|
||||
|
||||
// Update the lock field (this should trigger the notification)
|
||||
|
||||
@@ -489,6 +489,11 @@ pub async fn handle_dependency_job(
|
||||
// Since only worker that ran this Dependency Job has the cache
|
||||
// we do not need to think about invalidating cache for other workers.
|
||||
cache::script::invalidate(current_hash);
|
||||
// The version only became runnable now, so this process still resolves the path to
|
||||
// the one before it. Only the runnable-hash cache: the import-side caches ignore the
|
||||
// lock, so evicting this process' half of that pair here would key a bundle by a
|
||||
// hash whose content cache has not caught up.
|
||||
windmill_common::invalidate_deployed_script_hash_cache(w_id, script_path);
|
||||
|
||||
if let Err(e) = handle_deployment_metadata(
|
||||
&job.permissioned_as_email,
|
||||
|
||||
@@ -688,7 +688,7 @@ export const mcpEndpointTools: EndpointTool[] = [
|
||||
{
|
||||
name: "createScript",
|
||||
description: "create script: Creates a script at a path that does not already hold one",
|
||||
instructions: "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one.",
|
||||
instructions: "Specify the path (e.g., 'f/my_folder/my_script'), the content (source code), and the language. For TypeScript, use 'bun' unless deno-specific APIs are needed. A path that already holds a script is refused: use updateScript to deploy a new version of it, and do NOT delete and recreate a script to change it. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on.",
|
||||
path: "/w/{workspace}/scripts/create",
|
||||
method: "POST",
|
||||
pathParamsSchema: undefined,
|
||||
@@ -737,7 +737,7 @@ export const mcpEndpointTools: EndpointTool[] = [
|
||||
{
|
||||
name: "updateScript",
|
||||
description: "update script: Deploys a new version of the script at `path`, which must already hold one.\nThe body's `path` is the destination: the same path leaves the script where it\nis, a different one moves it there and archives the old path",
|
||||
instructions: "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async and can take up to a minute before a run by path uses it rather than the previous one.",
|
||||
instructions: "Deploys a new version of an existing script, preserving its history, so do NOT delete and recreate a script to change it. Send the whole script, not a patch: read the current one with getScriptByPath first, unless you wrote its content yourself. Set path__body only to move the script to a different path; omit it to leave the script where it is. A path that holds no script is refused: use createScript to create one. A new version generates its lock async, and only a version with a lock is runnable: until it lands, a run by path still executes the previous version. Poll getScriptByPath before running the new one and stop on either outcome: lock non-null means it is ready, lock_error_logs set means the lockfile failed and that version will never run, so report the error instead of polling on.",
|
||||
path: "/w/{workspace}/scripts/update/{path}",
|
||||
method: "POST",
|
||||
pathParamsSchema: {
|
||||
|
||||
Reference in New Issue
Block a user