✨ Add proxy option #136
Merged
✨ Add proxy option #136
Conversation
This file contains 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
Adding `proxy` command line option to specify proxy string as an argument. THe proxy s passed to analyzer providers settings. Fixes: konveyor#11 Signed-off-by: Marek Aufart <maufart@redhat.com>
|
This is looking great, can we make sure to have the NOPROXY settings as well as use environment variables |
Signed-off-by: Marek Aufart <maufart@redhat.com>
Signed-off-by: Marek Aufart <maufart@redhat.com>
shawn-hurley
reviewed
Jan 8, 2024
cmd/analyze.go
Outdated
| @@ -162,8 +163,9 @@ func NewAnalyzeCmd(log logr.Logger) *cobra.Command { | |||
| analyzeCommand.Flags().BoolVar(&analyzeCmd.overwrite, "overwrite", false, "overwrite output directory") | |||
| analyzeCommand.Flags().StringVar(&analyzeCmd.jaegerEndpoint, "jaeger-endpoint", "", "jaeger endpoint to collect traces") | |||
| analyzeCommand.Flags().BoolVar(&analyzeCmd.enableDefaultRulesets, "enable-default-rulesets", true, "run default rulesets with analysis") | |||
| analyzeCommand.Flags().StringVar(&analyzeCmd.proxy, "proxy", "", "HTTP&HTTPS proxy string URL") | |||
| analyzeCommand.Flags().StringVar(&analyzeCmd.noProxy, "no-proxy", "", "proxy excluded URLs (relevant only with `proxy`)") | |||
| analyzeCommand.Flags().StringVar(&analyzeCmd.httpProxy, "http-proxy", os.Getenv("http_proxy"), "HTTP proxy string URL") | |||
There was a problem hiding this comment.
one small concern, would be that the default value printed out here, would be changed based on env var.
Also I think you will need to handle both http_proxy and HTTP_PROXY.
There was a problem hiding this comment.
Thanks for comment! Printing actual default values in help didn't look bad to me.
What I've been looking on proxy env variables, lower case http_proxy is much more common than HTTP_PROXY. However, please update me if mentioned issues needs to be addressed.
There was a problem hiding this comment.
Pretty sure it does; Docker, I think, only uses uppercase, and I think Openshift does as well.
eemcmullan
approved these changes
Jan 9, 2024
Signed-off-by: Marek Aufart <maufart@redhat.com>
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.
Adding
proxycommand line option to specify proxy string as an argument. THe proxy s passed to analyzer providers settings.Fixes: #11