Skip to content

chore: remove middleware to request version and entitlement warnings #12750

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
merged 1 commit into from
Mar 25, 2024

Conversation

kylecarbs
Copy link
Member

@kylecarbs kylecarbs commented Mar 25, 2024

This cleans up root.go a bit, adds tests for middleware HTTP transport functions, and removes two HTTP requests we have always performed previously when executing any client command. It should improve CLI performance (especially for users with higher latency).

Version information was already sent as a header to every request. This adds entitlement warnings to requests if they exist. This seems reasonable because it's rare for users to have entitlement warnings.

The only behavior this breaks is when running coder logout; if you're already logged out, it will no longer error.

I removed a test that regressed in this commit. It's for the deprecated command, so it seems minimal risk since it wasn't working anyways.

@kylecarbs kylecarbs requested a review from mafredri March 25, 2024 17:24
@kylecarbs kylecarbs self-assigned this Mar 25, 2024
@kylecarbs kylecarbs force-pushed the vercheck branch 8 times, most recently from 590af3d to c9ea886 Compare March 25, 2024 18:19
@@ -67,8 +67,7 @@ const (
varOrganizationSelect = "organization"
varDisableDirect = "disable-direct-connections"

notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
notLoggedInURLSavedMessage = "You are not logged in. Try logging in using 'coder login'."
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login [url]'."

URL is sometimes optional, if we're only using on message then I'd prefer highlighting this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well the funny thing is for you to get this message, you have to already hit the server.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems like this shows the URL if you don't have it, and coder login if you do have it. That code wasn't really used properly!

This cleans up `root.go` a bit, adds tests for middleware HTTP transport
functions, and removes two HTTP requests we always always performed previously
when executing *any* client command.

It should improve CLI performance (especially for users with higher latency).
@kylecarbs kylecarbs merged commit 03ab37b into main Mar 25, 2024
@kylecarbs kylecarbs deleted the vercheck branch March 25, 2024 19:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants