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

fix(app): fix errant GET /run/:runId requests #16645

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Oct 30, 2024

Overview

When viewing the Devices tab on the desktop app, there are a lot of GET requests to /runs/null. These errant requests are generated by two places:

useRunStatuses

The complex enabled logic has bit us in the past here - in this case, the custom enabled condition overrides the default condition not to fetch if there is no runId, creating the errant fetch. Prior to notifications, it was important to selectively enable this hook to prevent polling unnecessarily. However, telemetry indicates that pretty much everyone uses notifications, and this hook uses notifications, so for the sake of keeping things simple, let's just remove this logic. Worst case scenario, it's not terrible to poll here if MQTT is blocked.

useNotifyRunQuery

A manual refetch() does not use the underlying enabled conditional logic of useRunQuery. There are two ways to solve it, either the way in this PR, or do extend useRunQuery to return a manual refetch function that wraps the custom, lower-level enabled logic.

For simplicity, I've chosen path 1, but I do think it would be worth revisiting this if we add more notification hooks with "always fetch only in specific circumstances" type logic.

Test Plan and Hands on Testing

  • On the desktop app, navigated to the Devices tab and noted that healthy reachable robots without active runs no longer make requests to runs/:runId.

Changelog

  • There are no longer errant network requests to /runs/runId when there is no active run.

Risk assessment

low - this doesn't touch networking in any potentially strange way.

@mjhuff mjhuff requested a review from a team October 30, 2024 19:27
@mjhuff mjhuff requested a review from a team as a code owner October 30, 2024 19:27
@mjhuff mjhuff requested review from shlokamin and removed request for a team October 30, 2024 19:27
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Easy once we work through the logic

@mjhuff mjhuff merged commit a74408d into chore_release-8.2.0 Oct 30, 2024
30 checks passed
@mjhuff mjhuff deleted the app_fix-errant-run-requests branch October 30, 2024 21:17
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.

2 participants