-
Notifications
You must be signed in to change notification settings - Fork 18
chore: display selected agents in UI #944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,53 @@ | ||
| import { trpc } from "~/api/trpc"; | ||
| import { Skeleton } from "~/components/ui/skeleton"; | ||
| import { useDeployment } from "../_components/DeploymentProvider"; | ||
| import { DeploymentAgentCard } from "./_components/DeploymentAgentCard"; | ||
|
|
||
| export default function DeploymentJobAgentPage() { | ||
| const { deployment } = useDeployment(); | ||
| const { data: agents, isLoading } = trpc.deployment.jobAgents.useQuery({ | ||
| deploymentId: deployment.id, | ||
| }); | ||
|
|
||
| return ( | ||
| <div className="m-8 max-w-3xl justify-center space-y-6"> | ||
| <div className="space-y-2"> | ||
| <h2 className="text-2xl font-bold">Job Agents</h2> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Job agents are used to dispatch jobs to the correct service. Without | ||
| an agent new deployment versions will not take any action. | ||
| Job agents matched by this deployment's selector. Without a matching | ||
| agent, new deployment versions will not take any action. | ||
| </p> | ||
| {deployment.jobAgentSelector != null && | ||
| deployment.jobAgentSelector !== "false" && ( | ||
| <div className="space-y-1"> | ||
| <h4 className="text-sm font-medium">Selector</h4> | ||
| <pre className="rounded-md bg-muted p-3 font-mono text-xs"> | ||
| {deployment.jobAgentSelector} | ||
| </pre> | ||
| </div> | ||
| )} | ||
|
Comment on lines
+20
to
+28
|
||
| </div> | ||
|
|
||
| {isLoading && ( | ||
| <div className="grid gap-4"> | ||
| <Skeleton className="h-32 w-full" /> | ||
| <Skeleton className="h-32 w-full" /> | ||
| </div> | ||
| )} | ||
|
|
||
| {!isLoading && agents != null && agents.length > 0 && ( | ||
| <div className="grid gap-4"> | ||
| {agents.map((agent) => ( | ||
| <DeploymentAgentCard key={agent.id} agent={agent} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
|
|
||
| {!isLoading && (agents == null || agents.length === 0) && ( | ||
| <p className="text-sm text-muted-foreground"> | ||
| No job agents match the current selector. | ||
| </p> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| local openapi = import '../lib/openapi.libsonnet'; | ||
|
|
||
| { | ||
| '/v1/deployments/{deploymentId}/job-agents': { | ||
| get: { | ||
| summary: 'Get job agents matching a deployment selector', | ||
| operationId: 'getJobAgentsForDeployment', | ||
| parameters: [ | ||
| openapi.deploymentIdParam(), | ||
| ], | ||
| responses: openapi.okResponse({ | ||
| type: 'object', | ||
| properties: { | ||
| items: { | ||
| type: 'array', | ||
| items: openapi.schemaRef('JobAgent'), | ||
| }, | ||
| }, | ||
| required: ['items'], | ||
| }, 'Job agents matching the deployment selector') | ||
| + openapi.badRequestResponse() | ||
| + openapi.notFoundResponse(), | ||
| }, | ||
| }, | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package deployments | ||
|
|
||
| import ( | ||
| "errors" | ||
| "net/http" | ||
|
|
||
| "github.com/gin-gonic/gin" | ||
| "github.com/google/uuid" | ||
| "github.com/jackc/pgx/v5" | ||
| "workspace-engine/pkg/db" | ||
| "workspace-engine/pkg/oapi" | ||
| "workspace-engine/pkg/selector" | ||
| ) | ||
|
|
||
| type Deployments struct{} | ||
|
|
||
| func (d *Deployments) GetJobAgentsForDeployment(c *gin.Context, deploymentId string) { | ||
| ctx := c.Request.Context() | ||
| queries := db.GetQueries(ctx) | ||
|
|
||
| deploymentUUID, err := uuid.Parse(deploymentId) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid deployment ID"}) | ||
| return | ||
| } | ||
|
|
||
| deployment, err := queries.GetDeploymentByID(ctx, deploymentUUID) | ||
| if err != nil { | ||
| if errors.Is(err, pgx.ErrNoRows) { | ||
| c.JSON(http.StatusNotFound, gin.H{"error": "Deployment not found"}) | ||
| return | ||
| } | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get deployment"}) | ||
| return | ||
| } | ||
|
Comment on lines
+27
to
+35
|
||
|
|
||
| allAgents, err := queries.ListJobAgentsByWorkspaceID(ctx, deployment.WorkspaceID) | ||
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list job agents"}) | ||
| return | ||
| } | ||
|
|
||
| oapiAgents := make([]oapi.JobAgent, len(allAgents)) | ||
| for i, row := range allAgents { | ||
| oapiAgents[i] = *db.ToOapiJobAgent(row) | ||
| } | ||
|
|
||
| matched, err := selector.MatchJobAgents(ctx, deployment.JobAgentSelector, oapiAgents) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Failed to evaluate selector: " + err.Error()}) | ||
| return | ||
| } | ||
|
|
||
| if matched == nil { | ||
| matched = []oapi.JobAgent{} | ||
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{"items": matched}) | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -571,4 +571,28 @@ export const deploymentsRouter = router({ | |
| }; | ||
| }); | ||
| }), | ||
|
|
||
| jobAgents: protectedProcedure | ||
| .input(z.object({ deploymentId: z.string() })) | ||
| .meta({ | ||
|
Comment on lines
+575
to
+577
|
||
| authorizationCheck: ({ canUser, input }) => | ||
| canUser | ||
| .perform(Permission.DeploymentGet) | ||
| .on({ type: "deployment", id: input.deploymentId }), | ||
| }) | ||
| .query(async ({ input }) => { | ||
| const result = await getClientFor().GET( | ||
| "/v1/deployments/{deploymentId}/job-agents", | ||
| { params: { path: { deploymentId: input.deploymentId } } }, | ||
| ); | ||
|
|
||
| if (result.error != null) { | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: `Failed to list job agents for deployment: ${JSON.stringify(result.error)}`, | ||
| }); | ||
|
Comment on lines
+589
to
+593
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not collapse upstream 4xx errors into This endpoint can legitimately return caller-facing errors (for example invalid ID / missing deployment). Mapping all failures to 500 makes client behavior and UI states misleading. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| return result.data.items; | ||
| }), | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle query errors separately from true empty results.
If the query fails, the current condition falls through to the empty-state message, which is misleading. Add an explicit error branch (
isError) before the empty-state branch.🛠️ Suggested UI-state fix
Also applies to: 46-50
🤖 Prompt for AI Agents