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

[WIP] Upgrading guzzle. #219

Closed
wants to merge 4 commits into from
Closed

Conversation

dblock
Copy link
Member

@dblock dblock commented Aug 7, 2024

Description

Upgrades Guzze and removes RingHTTP.

What works:

  • Happy path for GET, in samples, run composer run index.
samples> composer run index

> php index.php
opensearch: 2.15.0

What doesn't work:

  • Tests don't pass, many need to be rewritten.
  • Connection pool success/failure handling skipped.
  • Largely not backwards compatible.

Next:

  • Fixing tests to pass minimally.

Issues Resolved

Closes #216.

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.

dblock added 4 commits August 5, 2024 14:49
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
{
$response = null;
$async = isset($options['client']['future']) ? $options['client']['future'] : null;
if (is_null($async) || $async === false) {
do {
$result = $result->wait();
} while ($result instanceof FutureArrayInterface);
} while ($result instanceof Promise);
}
return $result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this return type from FutureArrayInterface|array (callable|array was wrong) to Promise|array will break BC for projects using the future client option to run in async mode as it changes the return type for them.

However, as the generated code does not give access to those options to the projects using them (generated endpoints class are the ones able to configure options), it seems like Client::request() is the only method actually allowing to actually run async. So maybe all the async support is a bunch of dead code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed.

This change is a bit over my head and I have made no to slow progress. If someone (you?) wants to take it over and complete it, it would be amazing.

@stof stof mentioned this pull request Nov 6, 2024
3 tasks
@stof
Copy link
Contributor

stof commented Nov 15, 2024

I suggest closing this in favor of #233 as supporting any PSR-18 client is better than forcing to use Guzzle (in a Symfony project, devs will likely want to use symfony/http-client instead as they already have it)

@dblock dblock closed this Nov 15, 2024
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.

[FEATURE] Out of the box New Relic PHP agent support
2 participants