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

PoC for PSR-17 #2525

Draft
wants to merge 2 commits into
base: 7.5
Choose a base branch
from
Draft

PoC for PSR-17 #2525

wants to merge 2 commits into from

Conversation

gmponos
Copy link
Member

@gmponos gmponos commented Dec 30, 2019

Current PR is only meant for discussion. It's purpose is not to be merged.

The main goals of this are the following:

  • We want to support PSR-17
  • We want developers to use the rich and useful RequestOptions of guzzle in order to create a request and send it use PSR-18 method.

How:

The biggest change here is that we extract applyOptions function into a separate function. By extracting the applyOptions to a separate function developers are able to create classes like in the following example:

class MyHandler
{

    /**
     * @var ClientInterface
     */
    private $client;

    /**
     * @var RequestFactoryInterface
     */
    private $factory;

    public function __construct(Psr\Http\Client\ClientInterface $client, RequestFactoryInterface $factory)
    {
        $this->client = $client;
        $this->factory = $factory;
    }

    public function sendEmail(string $email)
    {
        $request = $this->factory->createRequest('POST', 'www.sendemail.com');
        $options = [RequestOptions::QUERY => ['email' => $email]];
        $request = apply_http_options($request, $options);
        $this->client->sendRequest($request);
    }
}

in order to support this we need to merge #1890

After merging this it will allow us to remove these three lines of codes: https://github.com/guzzle/guzzle/pull/2525/files#diff-ff73e042e738204c6da009e2ed19f783L161

These options are re-applied twice and they might have a performance impact since we are initializing two Streams, when user passes body as an option.

Related mentions before:

#1914
#1886

The bad thing I'm experiencing with my team is the more verbose way to create an HTTP request. They seem to find it complicated.

#2186 (comment)

@gmponos
Copy link
Member Author

gmponos commented Dec 30, 2019

I have updated the description. If possible I would like some comments :)

@gmponos gmponos changed the title WIP PoC for PSR-17 PoC for PSR-17 Dec 30, 2019
@sagikazarmark
Copy link
Member

I find it weird that we encourage people to use standards like PSR-17, but we also suggest using our RequestOptions.

What's the point in using PSR-17 then? People will hardly use an entirely different PSR-7/PSR-17 package with our options.

@gmponos
Copy link
Member Author

gmponos commented Jan 3, 2020

People will hardly use an entirely different PSR-7/PSR-17 package with our options.

I will revert this.. Most probably ppl will not use PSR-7/PSR-17/PSR-18 if they have to build a request in a verbose way when they can simply use $client->get('uri', ['query' => ['key' => 'value']]);

Extracting apply_options makes the pain less.

@gnumoksha
Copy link

FYI about a year ago I've found it very difficult to get the team working with PSR clientes because they are too verbose, so I ended up creating a wrapper https://gist.github.com/gnumoksha/b35d97613dd729315d3dc1ba0449544a

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Jun 10, 2020

so I ended up creating a wrapper https://gist.github.com/gnumoksha/b35d97613dd729315d3dc1ba0449544a

Nice. :P That code is an example of where the new ClientTrait could be used. ;)

@GrahamCampbell
Copy link
Member

https://github.com/guzzle/guzzle/blob/master/src/ClientTrait.php

@stale
Copy link

stale bot commented Oct 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Oct 8, 2020
@Nyholm Nyholm added lifecycle/keep-open Issues that should not be closed and removed lifecycle/stale No activity for a long time labels Oct 9, 2020
@GrahamCampbell
Copy link
Member

#3135 has now been merged, replacing #1890.

@gmponos
Copy link
Member Author

gmponos commented May 15, 2023

Will try to re-initiate this.

Thanks @GrahamCampbell .. Please give me an ETA of 5 days.. if you don't hear from me please proceed with any other implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/keep-open Issues that should not be closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants