-
Notifications
You must be signed in to change notification settings - Fork 904
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
Conversation
590af3d
to
c9ea886
Compare
@@ -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>'." |
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.
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.
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.
Well the funny thing is for you to get this message, you have to already hit the server.
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.
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).
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.