Skip to content
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

fix: http proxy configuration #40

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

ahitacat
Copy link
Contributor

HTTPClient was setting a custom transport instead of using the Default, leading to a unconfigured proxy from environment variables.

This commit modifies the default transport to use the proxy env variables.

Resolves: RHBZ#2223405

Copy link
Collaborator

@subpop subpop left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

LGTM. I have only two small requests.

main.go Outdated Show resolved Hide resolved
internal/http/client.go Show resolved Hide resolved
HTTPClient was setting a custom transport instead of using the Default,
leading to a unconfigured proxy from environment variables.

This commit modifies the default transport to use the proxy env variables.

Signed-off-by: Alba Hita Catala <ahitacat@redhat.com>
@ahitacat
Copy link
Contributor Author

Changes

  • Addressed @jirihnidek 's comment:
    • Added the space that was removed in a prior change.
    • Added some comments to give an explanation about the use of the defaultTransporter.

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks for updates.

@jirihnidek jirihnidek merged commit 1990fb7 into RedHatInsights:main Jul 19, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants