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

PSR-7 compliance #357

Open
mschop opened this issue Sep 10, 2023 · 8 comments
Open

PSR-7 compliance #357

mschop opened this issue Sep 10, 2023 · 8 comments

Comments

@mschop
Copy link

mschop commented Sep 10, 2023

Hi,

I am trying to write an integration between a framework (https://packagist.org/packages/phespro/phespro) and amphp http server.

Unfortunately amphp/http-server does not follow PSR-7 (https://www.php-fig.org/psr/psr-7/).

The class \Amp\Http\Server\Request does not implement \Psr\Http\Message\ServerRequestInterface

This prevents me from simply forwarding the request to the kernel of that framework (which would have been a no brainer).

Why does amphp/http-server not follow that standard? Are there any plans to support PSR-7 in the future?

Thanks in advance!

Best Regards
Martin

@mschop mschop changed the title PSR compliance PSR-7 compliance Sep 11, 2023
@trowski
Copy link
Member

trowski commented Sep 15, 2023

Hi @mschop!

This library does not follow PSR-7 because the interfaces do not fit well into a non-blocking application and do not fit into the eco-system we've created in Amp using components such as ReadableStream, designed for non-blocking consumption.

However, with fibers improving integration of sync and async code, providing an adaptor should be an easy task. I started this some time ago in trowski/psr-http-bridge with v2 of this library and intend to update it to v3 and port the library to this project.

Ideally, because requests may perform blocking actions, clients would be spread amongst a pool of worker processes. This is on our roadmap, likely in another library using amphp/cluster

@raghuveer
Copy link

interested to see PSR-7 support for Amphp http server

@mschop
Copy link
Author

mschop commented Nov 19, 2023

@trowski Could you maybe add more detail on why PSR-7 does not fit well into this kind of non-blocking application?

\Psr\Http\Message\ServerRequestInterface and \Psr\Http\Message\ResponseInterface use a stream for the body.

Btw. ReactPHP HTTP server uses PSR-7.

@mschop
Copy link
Author

mschop commented Nov 19, 2023

Idea:

I will use react/http for handling incoming http connections with my framework. Inside of my application I will use revolt/event-loop-adapter-react for being able to use the amphp.

Do you think this is a suitable approach, or are there any drawbacks on this approach?

@kelunik
Copy link
Member

kelunik commented Nov 19, 2023

@mschop The doesn't fit well with non-blocking I/O is rather an argument of the past, it could work fine now. However, our current interface fits better with the amphp ecosystem, which has its own stream interface.

I'd be great if there was a proper stream interface PSR, but unfortunately, this was put into the HTTP PSR at that time and a bit too focused on the existing PHP SAPI.

You can of course also just use react/http and revolt/event-loop-adapter-react. It will be a different server implementation then and you'll loose features like HTTP/2 support, but that might not be too impoartant for an application server that's running behind a load balancer like Nginx.

We plan to eventually have a PSR-7 adpter on top of our current API, but we don't plan to make our API directly depend on PSR-7 here.

@mschop
Copy link
Author

mschop commented Nov 19, 2023

@kelunik Thank you for pointing out that react/http does not support HTTP/2. This is not ideal, but OK for now.

A PSR-7 adapter would be great 👍 .

@raghuveer
Copy link

We plan to eventually have a PSR-7 adpter on top of our current API, but we don't plan to make our API directly depend on PSR-7 here.

it will be really nice to have PSR-7 support through PSR-7 adapter

@luzrain
Copy link
Contributor

luzrain commented Oct 28, 2024

Any news about PSR-7 adapter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants