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.
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
feat: make proxy configurable #267
feat: make proxy configurable #267
Changes from all commits
eba288c
59352ec
52536b9
9cf61d3
67b18dd
28e59e6
a2ee797
b20b926
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Any reason to not make this (and kind of all the other functions) methods off of
QueryConfig
? To me, that's one of the main benefits of defining aQueryConfig
in the first place - you don't have to worry about passing the right arguments?I could be convinced otherwise though, currently I'm thinking as long as the public/external helper functions are methods that's good enough for me (e.g. skip this one and refactor
GetAWSV2Config
to be a method).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.
Agree that it makes sense, I was being a bit too careful with refactoring I suppose.
The following are now members of
QueryConfig
(and have been moved tocommon.go
:getCloudCredentials
-> publicgetCloudCredentialsFromBackplaneAPI
getCloudConsole
-> publicI also refactored
GetCloudConsole
the same wayGetCloudCredentials
has been refactored to enable calling it fromconsole.go
, and moved some logic into the newgetCloudConsoleFromBackplaneAPI
to make it less confusing.After these changes I tested the following:
Old flow:
Non STS clusters:
New flow: