fix: deprecate codersdk.AITaskPromptParameterName and reduce usage#20501
fix: deprecate codersdk.AITaskPromptParameterName and reduce usage#20501
Conversation
mafredri
force-pushed
the
mafredri/fix-coderd-replace-ai-promp
branch
4 times, most recently
from
483371e to
b3dd0e7
Compare
mafredri
requested review from
DanielleMaywood and
johnstcn
and removed request for
DanielleMaywood
| -- Consider all tasks, deleting a task does not turn the | ||
| -- workspace into a non-task workspace. | ||
| tasks.workspace_id = workspaces.id | ||
| ) |
There was a problem hiding this comment.
Review: This here breaks with un-patched sqlc because it puts tasks.owner_id in global scope even though it's not selected.
Should we fork sqlc for now?
There was a problem hiding this comment.
I'm fine with forking sqlc temporarily while an upstream bug is filed. I wonder if there's also the possibility of selecting id, workspace_id from tasks in a CTE and doing the join on that?
There was a problem hiding this comment.
Can you drop the original bugged sql query? I'm curious to see what the failing/bugged query looked like.
There was a problem hiding this comment.
@Emyrk This edit IS the bugged query, but check the added testdata in coder/sqlc#1 to see more.
| }). | ||
| WithAgent(). | ||
| WithTask(database.TaskTable{ | ||
| Prompt: prompt, | ||
| }, nil). | ||
| Do() | ||
|
|
||
| return build.Task |
| r.Route("/aitasks", func(r chi.Router) { | ||
| r.Use(apiKeyMiddleware) | ||
| r.Get("/prompts", api.aiTasksPrompts) | ||
| }) |
There was a problem hiding this comment.
I'm potentially concerned about this given our "one minor release" backward compat guarantee. Maybe mark this endpoint deprecated instead?
There was a problem hiding this comment.
Was this ever used by anything other than the frontend? Considering the route is experimental and you can't run an old coderd with a newer frontend, I imagined this should be fine.
There was a problem hiding this comment.
Not that I'm aware, no. I guess we can remove it with the provisio that this was always under /api/experimental.
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
Refs coder/sqlc#1 Unblocks #20501 Upstream sqlc-dev/sqlc#4159
mafredri
force-pushed
the
mafredri/fix-coderd-replace-ai-promp
branch
from
b3dd0e7 to
e592af3
Compare
4 participants
Depends on coder/sqlc#1
Fixes coder/internal#979
Updates coder/internal#973
Notes:
sqlc, without the patch it incorrectly identifies ambigious columns.queries/workspaces.sql:401:13: column reference "owner_id" is ambiguous(false positive)