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

Provide custom OkHttpClient for MetricsReporter #65

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

vitoksmile
Copy link
Contributor

About the changes

We have a way to set custom OkHttpClient for UnleashClient, however we are missing it for HttpMetricsReporter. Custom OkHttpClient is usefull when we want to add our own Interceptor (e.g. HttpLoggingInterceptor).

@vitoksmile vitoksmile force-pushed the metrics/http-client branch 2 times, most recently from 5c44638 to 360ebf5 Compare January 17, 2024 11:34
@vitoksmile
Copy link
Contributor Author

Test pipeline is failing

Execution failed for task ':coverallsJacoco'. > COVERALLS_REPO_TOKEN not set

@andreas-unleash
Copy link
Contributor

Happy to approve. Could use a test proving that the passed client is used for posting.

@andreas-unleash
Copy link
Contributor

Thanks for contributing :) I will look into the token error

@andreas-unleash
Copy link
Contributor

@vitoksmile Please merge main back to this branch so you get the new workflow (that doesn't run the coveralls on external PRs)

@vitoksmile
Copy link
Contributor Author

vitoksmile commented Jan 23, 2024

@andreas-unleash rebased into main

Copy link
Contributor

@andreas-unleash andreas-unleash 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!

@andreas-unleash
Copy link
Contributor

The failure is only in your fork. Maybe you need the workflows there as well

@vitoksmile
Copy link
Contributor Author

@andreas-unleash will you include this change into next release?

@andreas-unleash
Copy link
Contributor

Yes. Probably shortly

@andreas-unleash andreas-unleash merged commit a8acf75 into Unleash:main Jan 23, 2024
1 check 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.

2 participants