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.

Sign up for GitHub

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

Jump to bottom

Discovery Client: Do not attempt to "refresh" MDNS advertising status of a given robot #5985

Closed
1 task
mcous opened this issue Jun 23, 2020 · 1 comment · Fixed by #6193
Closed
1 task

Discovery Client: Do not attempt to "refresh" MDNS advertising status of a given robot #5985

mcous opened this issue Jun 23, 2020 · 1 comment · Fixed by #6193
Labels
app Affects the `app` project bug robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience).
Milestone
CPX Sprint 14

Comments

@mcous
Copy link
Contributor

mcous commented Jun 23, 2020

overview

The discovery client does this weird thing where it periodically tries to wipe out whether or not a given robot is actively advertising on MDNS and then re-queries, trying to fill that state back out. It doesn't work and mostly just leads to confusing (and wrong) warning messages in the app about "your robot is advertising its IP address but not responding to requests".

We should cut our losses and remove that weird refresh. Moving forward, the discovery client's advertising flag will simply mean "we have received an MDNS advertisement from this robot during this boot of the app/discovery-client".

acceptance critiera

  • Remove everything related to client._rediscover
@mcous mcous added bug app Affects the `app` project robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). labels Jun 23, 2020
@mcous mcous added this to the CPX Sprint 12 milestone Jun 23, 2020
@mcous mcous modified the milestones: CPX Sprint 12, CPX Sprint 13 Jul 7, 2020
@mcous
Copy link
Contributor Author

mcous commented Jul 14, 2020 •
edited
Loading

Update to this ticket: #6106 does a couple things that allow us to replicate the intent of the advertising flag (is this robot reachable at all?) in a more accurate manner:

  • Every IP address has a seen latch, which says: "I've seen this IP address via mDNS or a fetch that returned a status code" during this discovery session
  • Granular status enums for an IP's /health and /server/health endpoints that can be one of:
    • OK - Endpoint responded 2xx
    • Not OK - Endpoint responded, but with a 4xx or 5xx
    • Unreachable - Fetch was not able to complete

@mcous mcous modified the milestones: CPX Sprint 13, CPX Sprint 14 Jul 21, 2020
mcous added a commit that referenced this issue Jul 22, 2020
@mcous
fix(app): simplify discovery state to avoid stale health data
e578c3b 
Switches the app to a newer implementation of the Discovery Client with improved state management

Closes #5985
@mcous mcous mentioned this issue Jul 22, 2020
Merged
9 tasks
mcous added a commit that referenced this issue Aug 3, 2020
@mcous
fix(app): simplify discovery state to avoid stale IPs and health data (…
0e089a7 
…#6193)

Closes #5985, closes #5250
@mcous mcous mentioned this issue Apr 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
app Affects the `app` project bug robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience).
Projects
None yet
Milestone
CPX Sprint 14
Development

Successfully merging a pull request may close this issue.

fix(app): simplify discovery state to avoid stale IPs and health data Opentrons/opentrons
1 participant
@mcous