-
Notifications
You must be signed in to change notification settings - Fork 179
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
fix(app): Fix improper transitioning run status in protocol runs #14459
fix(app): Fix improper transitioning run status in protocol runs #14459
Conversation
…ropriately Instead of checking the refetchInterval property to see if a notification refetch should occur, we should check if staleTime is infinity. This accurately captures the refetchHTTP behavior that we actually want.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## chore_release-7.2.0 #14459 +/- ##
=======================================================
+ Coverage 67.70% 67.78% +0.08%
=======================================================
Files 1628 2519 +891
Lines 54904 72004 +17100
Branches 4147 9256 +5109
=======================================================
+ Hits 37172 48810 +11638
- Misses 17042 20991 +3949
- Partials 690 2203 +1513
Flags with carried forward coverage won't be shown. Click here to find out 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.
couple small change requests for python style and robustness
robot-server/robot_server/service/notifications/publishers/runs_publisher.py
Outdated
Show resolved
Hide resolved
robot-server/robot_server/service/notifications/publishers/runs_publisher.py
Outdated
Show resolved
Hide resolved
robot-server/robot_server/service/notifications/publishers/runs_publisher.py
Outdated
Show resolved
Hide resolved
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!
) Closes RQA-2291, RQA-2307, RQA-2306, RQA-2304 * fix(app): fix non polling notify hooks not always refetching data appropriately Instead of checking the refetchInterval property to see if a notification refetch should occur, we should check if staleTime is infinity. This accurately captures the refetchHTTP behavior that we actually want. * fix(app): fix infinite cancelling run state when run status is idle->stop-requested
Closes RQA-2291, RQA-2307, RQA-2306, RQA-2304
Overview
This PR fixes a bug on the app side in which the incorrect React Query property was used. In order to prevent any refetches, staleTime is the correct choice. Certain useRunQuery and useAllRunQuery hooks that were not updating properly now update as expected. Also, the removed
RUN_STATUS_STOP_REQUESTED
is added back to useRunStatus(), since its removal was causing bug RQA-2306.The python side changes are all sanity check changes - nothing functionally new here. A new push is added to the remove method, since we want to push on every new CRUD. I don't know if this was causing bugs, but it should be fixed.
Test Plan
Changelog
Risk assessment
low