mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 16:03:47 +00:00
fix(datatables): a lookup that failed is not an answer
An empty role list meant "nothing to pick" whether the endpoint said so or never answered, so a failed lookup on a permissioned data table let an app be created naming it with no role — the case the check exists for. The failure is carried, and an app that would name the data table waits for an answer. The fork dialog said "Keep original" on the same failure, for a data table the backend drops from the fork when its permissions are on: it says the check did not answer instead of asserting the outcome. The generated Python signature keeps the bare `*`. `datatable(name, *, role)` is keyword-only, and an agent following the advertised positional form gets a TypeError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n
This commit is contained in:
co-authored by
Claude Opus 5
parent
7c68818da1
commit
93590da9d8
@@ -141,10 +141,14 @@
|
||||
roles.current.permissioned &&
|
||||
loadedRoles.length === 0
|
||||
)
|
||||
// A lookup that failed is not an answer that there is nothing to pick, and an
|
||||
// app naming this data table would be created against whatever the default
|
||||
// turns out to be.
|
||||
const rolesUnknown = $derived(rolesSettled && roles.current.failed)
|
||||
// Only the app that will name the data table is refused: with table creation
|
||||
// off nothing saves it, and an app that does not touch it is not this caller's
|
||||
// problem to be stopped over.
|
||||
const blockedByRole = $derived(noUsableRole && tableCreationEnabled)
|
||||
const blockedByRole = $derived((noUsableRole || rolesUnknown) && tableCreationEnabled)
|
||||
const accessSettled = $derived(
|
||||
hasNoDatatables ||
|
||||
(rolesSettled &&
|
||||
@@ -361,9 +365,9 @@
|
||||
size="sm"
|
||||
class="w-40"
|
||||
/>
|
||||
{#if noUsableRole}
|
||||
{#if noUsableRole || rolesUnknown}
|
||||
<span class="text-xs text-red-600 dark:text-red-400">
|
||||
no role you can use
|
||||
{rolesUnknown ? 'could not read its roles' : 'no role you can use'}
|
||||
</span>
|
||||
{/if}
|
||||
{#if showRolePicker}
|
||||
|
||||
@@ -57,12 +57,17 @@ export function createRolesResource(
|
||||
* there is nothing to pick — which is not the same as a permissioned one
|
||||
* this caller may run as nothing on. */
|
||||
permissioned: boolean
|
||||
/** The lookup itself failed, so neither of the above is an answer: an
|
||||
* empty `roles` here means nothing was learned, not that there is
|
||||
* nothing to pick. */
|
||||
failed: boolean
|
||||
roles: string[]
|
||||
defaultRole: string
|
||||
}> => {
|
||||
const empty = {
|
||||
datatable: datatableName || undefined,
|
||||
permissioned: false,
|
||||
failed: false,
|
||||
roles: [],
|
||||
defaultRole: ADMIN_DATATABLE_ROLE
|
||||
}
|
||||
@@ -77,13 +82,14 @@ export function createRolesResource(
|
||||
}
|
||||
} catch (e) {
|
||||
console.error('Failed to load datatable roles:', e)
|
||||
return empty
|
||||
return { ...empty, failed: true }
|
||||
}
|
||||
},
|
||||
{
|
||||
initialValue: {
|
||||
datatable: undefined,
|
||||
permissioned: false,
|
||||
failed: false,
|
||||
roles: [],
|
||||
defaultRole: ADMIN_DATATABLE_ROLE
|
||||
}
|
||||
|
||||
@@ -58,10 +58,13 @@
|
||||
workspace: ws,
|
||||
datatableName: dt.name
|
||||
})
|
||||
return { ...dt, permissioned: roles.enabled }
|
||||
return { ...dt, permissioned: roles.enabled as boolean | undefined }
|
||||
} catch (e) {
|
||||
// Not `false`: what the fork does with this data table is decided by
|
||||
// the config, and saying "kept" for one the backend will drop loses
|
||||
// it silently.
|
||||
console.error('Failed to read datatable permissions:', e)
|
||||
return { ...dt, permissioned: false }
|
||||
return { ...dt, permissioned: undefined }
|
||||
}
|
||||
})
|
||||
)
|
||||
@@ -213,7 +216,12 @@
|
||||
items={[
|
||||
{
|
||||
value: 'keep_original',
|
||||
label: dt.permissioned ? 'Not shared (permissions enabled)' : 'Keep original'
|
||||
label:
|
||||
dt.permissioned === undefined
|
||||
? 'Keep original (permissions unknown)'
|
||||
: dt.permissioned
|
||||
? 'Not shared (permissions enabled)'
|
||||
: 'Keep original'
|
||||
},
|
||||
{ value: 'schema_only', label: 'Clone schema only' },
|
||||
...(!isCloudHosted() && $userStore?.is_admin
|
||||
|
||||
@@ -3052,7 +3052,7 @@ Import: \`import wmill\`
|
||||
#
|
||||
# Example:
|
||||
# wmill.datatable("main", role="operator")
|
||||
def datatable(name: str = 'main', role: Optional[str] = None) -> DataTableClient
|
||||
def datatable(name: str = 'main', *, role: Optional[str] = None) -> DataTableClient
|
||||
|
||||
# Client for executing SQL queries against Windmill DataTables.
|
||||
class DataTableClient:
|
||||
|
||||
@@ -14,7 +14,7 @@ Import: `import wmill`
|
||||
#
|
||||
# Example:
|
||||
# wmill.datatable("main", role="operator")
|
||||
def datatable(name: str = 'main', role: Optional[str] = None) -> DataTableClient
|
||||
def datatable(name: str = 'main', *, role: Optional[str] = None) -> DataTableClient
|
||||
|
||||
# Client for executing SQL queries against Windmill DataTables.
|
||||
class DataTableClient:
|
||||
|
||||
@@ -1265,6 +1265,10 @@ def _format_py_params(node: ast.FunctionDef, skip_self: bool = False) -> str:
|
||||
if args.vararg.annotation:
|
||||
vararg_str += f": {ast.unparse(args.vararg.annotation)}"
|
||||
params.append(vararg_str)
|
||||
elif args.kwonlyargs:
|
||||
# The bare separator is part of the signature: without it the advertised
|
||||
# call is positional, and an agent following it gets a TypeError.
|
||||
params.append('*')
|
||||
|
||||
for i, arg in enumerate(args.kwonlyargs):
|
||||
param_str = arg.arg
|
||||
|
||||
Reference in New Issue
Block a user