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

Adds http client factories #271

Merged
merged 2 commits into from
Jan 30, 2025
Merged

Conversation

kimpepper
Copy link
Collaborator

@kimpepper kimpepper commented Jan 28, 2025

Description

Adds http client factories with some useful default configuration to simplify http client creation.

Issues Resolved

#257

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kim Pepper <[email protected]>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Looks good, needs updates to the user guide.

src/OpenSearch/GuzzleHttpClientFactory.php Outdated Show resolved Hide resolved
@dblock
Copy link
Member

dblock commented Jan 28, 2025

Rebase after #272.

@kimpepper kimpepper force-pushed the client-builder branch 2 times, most recently from 1267d71 to fd1adb5 Compare January 29, 2025 08:05
Copy link
Collaborator Author

@kimpepper kimpepper left a comment

Choose a reason for hiding this comment

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

Thanks for the reviews. I think this covers all your feedback.

src/OpenSearch/GuzzleHttpClientFactory.php Outdated Show resolved Hide resolved
src/OpenSearch/GuzzleHttpClientFactory.php Outdated Show resolved Hide resolved
@kimpepper kimpepper changed the title Adds client factories Adds http client factories Jan 29, 2025
@kimpepper
Copy link
Collaborator Author

We should update the docs to use these.

@dblock
Copy link
Member

dblock commented Jan 30, 2025

We should update the docs to use these.

I'll merge, but do open an issue to remember to do it at least.

@dblock dblock merged commit 6e63822 into opensearch-project:main Jan 30, 2025
46 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.

3 participants