Skip to content

view: Wait for Auspice to start responding before opening the browser#291

Merged
tsibley merged 2 commits intomasterfrom
trs/view/wait-for-auspice
Jun 16, 2023
Merged

view: Wait for Auspice to start responding before opening the browser#291
tsibley merged 2 commits intomasterfrom
trs/view/wait-for-auspice

Conversation

@tsibley
Copy link
Contributor

@tsibley tsibley commented Jun 15, 2023

A big improvement over simply pausing for 2s before opening it and hoping for the best.

Testing

  • Manually, timing is right, and the timeouts work when I do nextstrain view --exec cat /var/empty
  • Checks pass

A big improvement over simply pausing for 2s before opening it and
hoping for the best.
@tsibley tsibley requested a review from a team June 15, 2023 23:59
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

The 2s timeout worked 50% of the time for me, so this is much better 😄

@tsibley
Copy link
Contributor Author

tsibley commented Jun 16, 2023

CI failure of a single job looks transient, but I'm re-running it now to see.

@tsibley
Copy link
Contributor Author

tsibley commented Jun 16, 2023

The non-changelog commit was successful after the re-attempt, so going to merge.

@tsibley tsibley merged commit 0ea50c7 into master Jun 16, 2023
@tsibley tsibley deleted the trs/view/wait-for-auspice branch June 16, 2023 20:08
@tsibley
Copy link
Contributor Author

tsibley commented Sep 14, 2023

Unrelated discussion of nextstrain view in Slack led to me realizing that there's a failure mode here in merely waiting for the expected Auspice URL to start listening: if the port is already in use by another HTTP server (Auspice or otherwise), then the code will assume its the Auspice it launched from the current process and open it in the browser, even though in reality the launched Auspice errors out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

2 participants