Skip to content

i15-1: display the queue from the daq-queue-service#45

Draft
noemifrisina wants to merge 48 commits into
mainfrom
1625-display-queue
Draft

i15-1: display the queue from the daq-queue-service#45
noemifrisina wants to merge 48 commits into
mainfrom
1625-display-queue

Conversation

@noemifrisina
Copy link
Copy Markdown
Collaborator

@noemifrisina noemifrisina commented May 27, 2026

Built on top of #44, we should merge that first

Copy link
Copy Markdown
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great start, thank you. I think we should probably see if @zoharma and @douglaswinter have opinions too. Some comments in code. Additionally, my initial thoughts on the look/feel, feel free to spin into new issues/PR given the size of this PR already:

  • Should: There's no feedback when we can't connect to the queue, it just says it's running
  • Should: I don't really think ID is useful to be user facing
  • Should: The pause will pause after the next task in the queue is finished, I think it would be good to rename it to make that more obvious
  • Should: This PR includes stuff for the abort button, we should probably have the same button on the queue page (in fact maybe everywhere)
  • Could: It would be nicer if the drag and drop was entirely disabled for the current/past tasks so its more obvious you can't do it
  • Could: If there's nothing in the queue then having it saying it's running is a bit odd, maybe "Queue finished"?
  • Could: I think some tooltips on the column headers would be good
  • Nit: You can filter by the action column, this doesn't really make much sense
  • Nit: Status is an enum but when you filter by it you put in a string, might be nicer if the filter was an enum

Comment thread apps/i15-1/src/queue/tasks.ts Outdated

export type TaskStatus =
| "Waiting"
| "Claimed"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I trigger a claimed state in the queue?

Copy link
Copy Markdown
Collaborator

@jacob720 jacob720 Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is leftover from before the queue refactor, I need to remove that type now as the correct one exists in generated/. Tasks can now only have the following status:

class Status(StrEnum):
    QUEUED = "Queued"
    IN_PROGRESS = "In progress"
    COMPLETE = "Complete"
    CANCELLED = "Cancelled"

Blueapi calls can be claimed though. This represents the time when the worker has popped the call from the queue but blueapi hasn't started running it yet. A task's status is derived from the status of the blueapi calls it has produced, so both a claimed and in_progress blueapi call make its parent task in_progress.

class CallStatus(StrEnum):
    WAITING = "Waiting"  # Waiting in the queue
    CLAIMED = "Claimed"  # Claimed by the worker
    IN_PROGRESS = "In progress"  # In progress inside BlueAPI
    SUCCESS = "Success"  # Completed successfully
    ERROR = "Error"  # Error while trying to run

Comment thread apps/i15-1/src/routes/QueueView.tsx Outdated
Comment thread apps/i15-1/src/routes/QueueView.tsx Outdated
Comment thread apps/i15-1/src/queue/tableData.ts Outdated
sampleId: string;
planRunning: string;
parameters: string;
// parameters: PlanParameters;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should: Remove commented out code

Comment thread apps/i15-1/src/queue/queueService.ts Outdated
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";

// const QUEUE_SOCKET: string = "/api/daq-queue";
const QUEUE_SOCKET: string = "http://127.0.0.1:8001";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must: Can we have some way of generically configuring this so that it works for testing and prod

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added an env variable to toggle between these, which also turns mock service worker on/off.

Comment thread apps/i15-1/src/queue/queueService.ts Outdated
Comment thread apps/i15-1/src/routes/QueueView.tsx Outdated
enableRowDragging: true,
enableSorting: false,
muiRowDragHandleProps: ({ table }) => ({
onDragEnd: () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should: I think some tests on the logic here would be good.

@douglaswinter
Copy link
Copy Markdown
Collaborator

Two top-level comments/questions: 1) How does this view relate to the design in Figma? 2) Echoing Dom's comment, we need tests for this, both plain unit tests for your queries and logic, and RTL tests for the rendering.

@jacob720 jacob720 force-pushed the 1625-display-queue branch from be4a094 to 6a0c321 Compare June 2, 2026 12:13
@jacob720 jacob720 force-pushed the 1625-display-queue branch from 6a0c321 to a839a06 Compare June 2, 2026 12:16
@jacob720
Copy link
Copy Markdown
Collaborator

jacob720 commented Jun 4, 2026

It's getting iteratively closer to the figma design, but still some differences:

  • Task params are all listed as a string rather than in individual columns as per the design i15-1: Investigate best way to display task params in queue  #52
  • Currently can't get estimated run time from blueapi so run time, progress columns don't yet exist
  • Currently have a toggle to show historic tasks instead of having tabs for future, historic, all, tasks
  • Currently tasks can always be re-ordered, there's a toggle in the design
  • Can't select tasks to perform actions on them - there's just a cancel button
  • The pause/resume queue button and queue status text look a bit different to the design

In the interest of getting this merged + deployed asap, so the scientists can have a play around with it, I can spin these into separate issues to fix in future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants