-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Patch to make the HTTPClient proxy-aware #2821
Conversation
@JorTurFer, apologies. I screwed up on the previous patch. quite a few files got tagged when I reset the branch. Resubmitted here. Will add PR for keda-docs shortly. |
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.
LGTM
❤️ ❤️ ❤️
keda-docs PR submitted #730 |
@JorTurFer, regarding your Helm chart question: I don't think it's necessary, as those who work with proxies will know by now how to add the environment variables to a chart's env section. Thanks again for the guidance! |
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.
Looking good, the code just needs formatting :)
@wltbenade would you mind fixing the formatting, please? |
hi @wltbenade |
Yeah, you should rebase, your branch is way too behind the main. |
Signed-off-by: Louwrens Benade <[email protected]>
Done :)
Done :)
and done :) |
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.
LGTM, thanks for the contribution!
Signed-off-by: Louwrens Benade [email protected]
Provide a description of what has been changed
Checklist
Fixes #2577
This patch addresses #2577