-
Notifications
You must be signed in to change notification settings - Fork 789
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
Try to fix stale queries when tasks and workflows are created #1169
Conversation
…src/' <!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Fix stale queries by invalidating and refetching data for tasks and workflows upon status changes and component mounts. > > - **Behavior**: > - In `TaskActions.tsx` and `WorkflowRun.tsx`, added `queryClient.invalidateQueries` to refresh queries for tasks and workflows when their status changes to completed, failed, or terminated. > - Changed `refetchOnMount` to "always" in `QueuedTasks.tsx`, `RunningTasks.tsx`, `WorkflowPage.tsx`, and `Workflows.tsx` to ensure data is always refetched when components mount. > - **Misc**: > - Updated dependencies in `TaskActions.tsx` and `WorkflowRun.tsx` to include `queryClient` for useEffect hooks. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for f977d9025076fede8b3956661a7bd563b65a8764. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
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.
👍 Looks good to me! Incremental review on 11bc249 in 20 seconds
More details
- Looked at
176
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx:34
- Draft comment:
UsingrefetchOnMount: "always"
ensures data freshness but may lead to unnecessary network requests. Consider the trade-off between data freshness and performance. This applies to other files as well. - Reason this comment was not posted:
Confidence changes required:50%
The use ofrefetchOnMount: "always"
is consistent across multiple files. This setting ensures that data is always refetched when the component mounts, which is a valid approach to ensure data freshness. However, it might lead to unnecessary network requests if the data doesn't change often. It's important to consider the trade-off between data freshness and performance.
Workflow ID: wflow_v0fwwCGSr1Iogk1B
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Reviewed everything up to 11bc249 in 1 minute and 30 seconds
More details
- Looked at
176
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx:34
- Draft comment:
refetchOnMount
should be a boolean, not a string. Usetrue
instead of "always". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment suggests a change that contradicts the documentation of the library being used. The@tanstack/react-query
library allowsrefetchOnMount
to be set to "always", which means the comment is likely incorrect.
I might be missing some context about a specific version of the library where this behavior is different. However, the documentation for the current version supports the use of "always".
Given the information available, the documentation supports the use of "always". If there were a version-specific issue, it would likely be documented or caught by tests.
The comment is incorrect because the@tanstack/react-query
library allowsrefetchOnMount
to be set to "always". The comment should be deleted.
2. skyvern-frontend/src/routes/tasks/running/RunningTasks.tsx:34
- Draft comment:
refetchOnMount
should be a boolean, not a string. Usetrue
instead of "always". - Reason this comment was not posted:
Marked as duplicate.
3. skyvern-frontend/src/routes/workflows/WorkflowPage.tsx:55
- Draft comment:
refetchOnMount
should be a boolean, not a string. Usetrue
instead of "always". - Reason this comment was not posted:
Marked as duplicate.
4. skyvern-frontend/src/routes/workflows/WorkflowRun.tsx:138
- Draft comment:
refetchOnMount
should be a boolean, not a string. Usetrue
instead of "always". - Reason this comment was not posted:
Marked as duplicate.
5. skyvern-frontend/src/routes/workflows/Workflows.tsx:96
- Draft comment:
refetchOnMount
should be a boolean, not a string. Usetrue
instead of "always". - Reason this comment was not posted:
Marked as duplicate.
6. skyvern-frontend/src/routes/tasks/detail/TaskActions.tsx:103
-
Draft comment:
This code duplicates existingqueryClient.invalidateQueries
calls withqueryKey: ['tasks']
found inCreateNewTaskForm.tsx
andTaskDetails.tsx
. Consider refactoring to avoid duplication. -
queryClient.invalidateQueries (CreateNewTaskForm.tsx)
-
queryClient.invalidateQueries (TaskDetails.tsx)
-
Reason this comment was not posted:
Based on historical feedback, this comment is too similar to comments previously marked by users as bad.
Workflow ID: wflow_NId5gbCvvcAu16uo
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Fix stale queries by invalidating and refetching data for tasks and workflows upon status changes and component mounts.
TaskActions.tsx
andWorkflowRun.tsx
, addedqueryClient.invalidateQueries
to refresh queries for tasks and workflows when their status changes to completed, failed, or terminated.refetchOnMount
to "always" inQueuedTasks.tsx
,RunningTasks.tsx
,WorkflowPage.tsx
, andWorkflows.tsx
to ensure data is always refetched when components mount.TaskActions.tsx
andWorkflowRun.tsx
to includequeryClient
for useEffect hooks.This description was created by for 11bc249. It will automatically update as commits are pushed.