mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(datatables): keep the fork schema baseline, and bounce streams on every removal
Three fixes from review. `edit_datatable_config` took `forked_from` wholesale from the stored entry, so the fork schema diff's save of an advanced baseline was silently discarded and an applied change was offered again. Whether an entry carries a clone stamp is still carried from the store, since that is what marks its database droppable, but the baseline inside it is now taken from the request. The stranded-pointer warning and the stream bounce ran over the optional `deleted_datatables` hint, which the settings-sync CLI never sends, so removing a governing data table through `wmill` bounced nothing. Removals are now derived from the stored configuration against the saved one. `delete_workspace` read the pointers to bounce before its transaction, so a fork committing a pointer during the deletion was missed. The read now happens inside the transaction, after the workspace row is deleted: a fork's insert key-share locks that row through its parent foreign key, so it is either seen or fails on the missing parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
8255f2ad2b
commit
dc3ebfbe74
@@ -3799,15 +3799,18 @@ async fn edit_datatable_config(
|
||||
Some(true)
|
||||
}
|
||||
};
|
||||
// Three fields this form does not own, carried across from the stored entry rather than
|
||||
// taken from the request. `permissions` is an access decision, edited through its own
|
||||
// endpoint; `reference` is what makes a fork answer to the workspace that governs its data
|
||||
// table, and letting a save clear it would hand the fork the database outright; and
|
||||
// `forked_from` is the clone stamp the fork flow writes. Only fork creation writes any of
|
||||
// them, so a settings save can neither widen nor lose them.
|
||||
// Carried across from the stored entry rather than taken from the request. `permissions`
|
||||
// is an access decision, edited through its own endpoint; `reference` is what makes a fork
|
||||
// answer to the workspace that governs its data table, and letting a save clear it would
|
||||
// hand the fork the database outright. `forked_from` is the clone stamp the fork flow
|
||||
// writes: whether an entry has one is carried the same way, since it is what marks the
|
||||
// database droppable, but the schema baseline inside it is the diff view's to advance.
|
||||
dt.permissions = old.and_then(|old| old.permissions.clone());
|
||||
dt.reference = old.and_then(|old| old.reference.clone());
|
||||
dt.forked_from = old.and_then(|old| old.forked_from.clone());
|
||||
dt.forked_from = match old.and_then(|old| old.forked_from.as_ref()) {
|
||||
Some(stored) => Some(dt.forked_from.take().unwrap_or_else(|| stored.clone())),
|
||||
None => None,
|
||||
};
|
||||
// Carrying the block onto a resource-backed entry would produce a data table the chokepoint
|
||||
// refuses on every job — a save that succeeds and breaks everything afterwards. Refuse it
|
||||
// instead: turning roles off first is one step, and it keeps discarding an access decision
|
||||
@@ -3868,6 +3871,18 @@ async fn edit_datatable_config(
|
||||
}
|
||||
}
|
||||
|
||||
// Every entry this save removes, derived rather than taken from `deleted_datatables`: that list
|
||||
// is a hint the settings-sync CLI never sends, and the stranded-pointer warning and the stream
|
||||
// bounce below must run for a removal whether or not the caller named it.
|
||||
let removed: Vec<String> = old_datatables
|
||||
.keys()
|
||||
.filter(|name| {
|
||||
!new_config.settings.datatables.contains_key(*name)
|
||||
&& !new_config.renames.iter().any(|r| &r.from == *name)
|
||||
})
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
let config: serde_json::Value = serde_json::to_value(new_config.settings)
|
||||
.map_err(|err| Error::internal_err(err.to_string()))?;
|
||||
|
||||
@@ -3907,7 +3922,7 @@ async fn edit_datatable_config(
|
||||
// A deletion cannot be followed the same way — there is nothing to point at any more. Read who
|
||||
// is left stranded so the caller is told, the way deleting a workspace does.
|
||||
let mut stranded: Vec<StrandedReference> = Vec::new();
|
||||
for name in &new_config.deleted_datatables {
|
||||
for name in &removed {
|
||||
let rows = sqlx::query!(
|
||||
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!"
|
||||
FROM workspace_settings ws
|
||||
@@ -3932,8 +3947,7 @@ async fn edit_datatable_config(
|
||||
// a listener that reconnects finds the entry gone instead of streaming on.
|
||||
crate::datatable_permissions::restart_streams_named(
|
||||
&mut *tx,
|
||||
new_config
|
||||
.deleted_datatables
|
||||
removed
|
||||
.iter()
|
||||
.map(|name| (w_id.clone(), name.clone()))
|
||||
.chain(
|
||||
|
||||
Reference in New Issue
Block a user