Skip to content
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

feat: update queue item on session completion #6408

Conversation

psychedelicious
Copy link
Collaborator

Summary

feat(app): update queue item's session on session completion

The session is never updated in the queue after it is first enqueued. As a result, the queue detail view in the frontend never never updates and the session itself doesn't show outputs, execution graph, etc.

We need a new method on the queue service to update a queue item's session, then call it before updating the queue item's status.

Queue item status may be updated via a session-type event or queue-type event. Adding the updated session to all these events is a hairy - simpler to just update the session before we do anything that could trigger a queue item status change event:

  • Before calling emit_session_complete in the processor (handles session error, completed and cancel events and the corresponding queue events)
  • Before calling cancel_queue_item in the processor (handles another way queue items can be canceled, outside the session execution loop)

When serializing the session, both in the new service method and the get_queue_item endpoint, we need to use exclude_none=True to prevent unexpected validation errors.

Related Issues / Discussions

Closes #5933

QA Instructions

  • On main, generate an image & wait for it to finish
  • Check the queue tab and expand the queue item detail view
  • Observe that the execution graph, results and such are empty
  • Check out this PR
  • Do the same thing
  • Observe that the queue item detail view shows the executed session
  • Invoke a workflow that will error (e.g. integer primitive of 0 into a resize image node's width)
  • Observe that the queue item detail view shows a partially executed session w/ error

Merge Plan

This is based on #5748. I'll merge and update this PR as required after that one is merged.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

The session is never updated in the queue after it is first enqueued. As a result, the queue detail view in the frontend never never updates and the session itself doesn't show outputs, execution graph, etc.

We need a new method on the queue service to update a queue item's session, then call it before updating the queue item's status.

Queue item status may be updated via a session-type event _or_ queue-type event. Adding the updated session to all these events is a hairy - simpler to just update the session before we do anything that could trigger a queue item status change event:
- Before calling `emit_session_complete` in the processor (handles session error, completed and cancel events and the corresponding queue events)
- Before calling `cancel_queue_item` in the processor (handles another way queue items can be canceled, outside the session execution loop)

When serializing the session, both in the new service method and the `get_queue_item` endpoint, we need to use `exclude_none=True` to prevent unexpected validation errors.
@psychedelicious psychedelicious marked this pull request as draft May 20, 2024 08:34
@github-actions github-actions bot added api python PRs that change python files services PRs that change app services frontend PRs that change frontend files labels May 20, 2024
@psychedelicious
Copy link
Collaborator Author

Marked as draft to prevent premature merge

This query is only subscribed-to in the `QueueItemDetail` component - when is rendered only when the user clicks on a queue item in the queue. Invalidating this tag instead of optimistically updating it won't cause any meaningful change to network traffic.
@psychedelicious
Copy link
Collaborator Author

superseded by #6419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api frontend PRs that change frontend files python PRs that change python files services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant