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

kakkoyun commented Apr 12, 2022

@yolossn Could you also handle DCO failures?

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

kakkoyun commented Apr 12, 2022

@yeya24 @dsonck92 Does this fulfill your requirements?

Copy link
Member

@kakkoyun kakkoyun left a comment

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

kakkoyun commented Apr 21, 2022

@yolossn PTAL yolossn#1

@kakkoyun kakkoyun requested a review from bwplotka Apr 21, 2022
@kakkoyun
Copy link
Member

kakkoyun commented Apr 21, 2022

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

@bwplotka
Copy link
Member

bwplotka commented Apr 21, 2022

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

kakkoyun commented Apr 29, 2022

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 Apr 29, 2022
@kakkoyun
Copy link
Member

kakkoyun commented Apr 29, 2022

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
4 checks passed
@bwplotka
Copy link
Member

bwplotka commented Apr 29, 2022

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

bwplotka commented Apr 29, 2022

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