-
Notifications
You must be signed in to change notification settings - Fork 16
Add status and start/stop buttons to recents view #243
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This way we can create multiple clients on the recent workspaces page without having to do the whole init thing for each one.
We need this information so we can query the status of recent connections as they could belong to multiple deployments. This could end up desyncing if the user manually edits their config file and changes the global config path in ProxyCommand. The alternative would be to parse the SSH config to make sure we have the right config directory but that would mean parsing ProxyCommand to extract the value of --global-config. As a fallback for connections that already exist and are not yet stored with the config directory we could split the host name itself on `--` since it has the domain in it and join with the default directory but this could be inaccurate if the default has been changed or if in the future we change the host name format.
So we can match against the API response. We could split the hostname on `--` but there are cases where that will fail (when the name or domain itself contains -- in specific configurations). We have to add the config path anyway so this is the best opportunity to add more information.
I guess this could happen if you manually edit the recents? In any case if there is no host name I am not sure there is value in trying to show the connection and it is making it difficult to check if the workspace is up.
We will need this in the recent connections view as well.
So we can show just the icon in the recent connections view.
750eae1
to
720e69f
Compare
This relies on some new data being stored with the recent connections so old connections will be in an unknown state.
johnstcn
reviewed
May 8, 2023
src/main/kotlin/com/coder/gateway/models/RecentWorkspaceConnection.kt
Outdated
Show resolved
Hide resolved
johnstcn
approved these changes
May 8, 2023
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.
We may need to worry about agent names as well as workspace names in future, but for now 👍
Good to know, thanks for the heads up!! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am not 100% on the design, I just stuck the start/stop buttons next to the existing terminal button, moved them to the right, and put an icon to the left of the host name to indicate status (it also has hover tooltip text). I think it could be greatly improved.
Might be easiest to review by commit, there is also some context in each description.