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

client: Allow configuration of http client #1025

Merged
merged 3 commits into from Apr 29, 2022

Conversation

yolossn
Copy link
Contributor

@yolossn yolossn commented Apr 11, 2022

Fixes #932

Signed-off-by: yolossn nssvlr@gmail.com

@yolossn yolossn changed the title client: Allow configuration of Client client: Allow configuration of http client Apr 11, 2022
api/client.go Outdated Show resolved Hide resolved
@kakkoyun
Copy link
Member

@yolossn Could you also handle DCO failures?

@yolossn yolossn force-pushed the config_client branch 2 times, most recently from 9ac3af8 to efe94be Compare April 12, 2022 13:09
@yolossn yolossn requested a review from kakkoyun April 12, 2022 13:09
@kakkoyun
Copy link
Member

@yeya24 @dsonck92 Does this fulfill your requirements?

Copy link
Member

@kakkoyun kakkoyun 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 the contribution.

api/client.go Show resolved Hide resolved
Signed-off-by: yolossn <nssvlr@gmail.com>
@dsonck92
Copy link

dsonck92 commented Apr 18, 2022

@yeya24 @dsonck92 Does this fulfill your requirements?

Considering it's nearly the same (barring some pointer stuff and comments) as the potential implementation, yes 😄

Update config documentation

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Member

@yolossn PTAL yolossn#1

@kakkoyun kakkoyun requested a review from bwplotka April 21, 2022 08:27
@kakkoyun
Copy link
Member

@bwplotka Any ideas why CircleCI is not properly functioning? Any way to re-run the checks?

@bwplotka
Copy link
Member

No idea, unfortunately. @yolossn do you mind pushing (even empty commit) ?

Add api.Config validation to prevent confusion
@yolossn
Copy link
Contributor Author

yolossn commented Apr 21, 2022

@bwplotka looks like the maintainer has to click Approve and Run since this is a first time contribution.

image

@kakkoyun
Copy link
Member

Somehow, CircleCI ignores this PR. I'll merge it and if builds fail I'll fix it.

@kakkoyun kakkoyun removed the request for review from bwplotka April 29, 2022 05:40
@kakkoyun
Copy link
Member

I can't even do that @bwplotka could you please try to merge this?

@bwplotka bwplotka merged commit 4048091 into prometheus:main Apr 29, 2022
@bwplotka
Copy link
Member

Sorry @yolossn, sounds like it is approve to run problem, but it got stuck for us
image

If @kakkoyun wants to help, I merged as admin (:

@bwplotka
Copy link
Member

Thanks!

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.

Expose, or allow inserting, the http.Client used by the api.Client
4 participants