Skip to content

coder ping UX concerns #14752

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

Closed
ammario opened this issue Sep 20, 2024 · 3 comments · Fixed by #14762
Closed

coder ping UX concerns #14752

ammario opened this issue Sep 20, 2024 · 3 comments · Fixed by #14762

Comments

@ammario
Copy link
Member

ammario commented Sep 20, 2024

  1. The standard ping command runs forever. coder ping goes for 10 round trips by default, which seems arbitrary and often frustrates me. ping running forever is a feature not a bug. It means I can leave it up on a terminal to monitor my network conditions for a long time. When I ctrl+c on coder ping I want a statistical summary just like ping.
  2. Related, the new diagnostics info introduced feat(cli): add p2p diagnostics to ping #14426 should go at the beginning of the ping log and not the end. When placed at the end of the log, the feature becomes more and more useless as -n approaches infinity.

cc @ethanndickson

@coder-labeler coder-labeler bot added the chore label Sep 20, 2024
@ammario ammario added bug and removed chore labels Sep 20, 2024
@ethanndickson
Copy link
Member

ethanndickson commented Sep 23, 2024

These both sound reasonable, although this would of course mean any diagnostics we determine would always get shown. We currently don't show them if a direct connection was established, as to avoid overloading the user with warnings when their connection is already preferable. We could continue to do that, but the diagnostics would need to come after the pings + ctrl+c?

@deansheather
Copy link
Member

Since we need to do pings to determine whether we got p2p or not we definitely need to put the diagnostics last. We can make them get written after you hit ctrl+c or after the -n count is reached.

@ammario
Copy link
Member Author

ammario commented Sep 23, 2024

Can we do one silent ping to show diagnostics and then print the rest?

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 a pull request may close this issue.

3 participants