-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Socket issues #57
Comments
It looks like Plesk have forked They have renamed the package As Plesk are well known in this space, I'm happy to continue with their fork for the time being unless php-http/socket-client becomes active again. We also have the option of either taking over maintenance of I'll make the change for this now. |
Ok so I've:
|
Looking at the code it appears that we don't support the http schema when using the socket client (it looks like an issue even in the old upstream repo). Trying to use http results in this issue: #56 (comment) If we do want to support connection over http (as shown in https://docs.docker.com/config/daemon/remote-access/), we should also include a @flavioheleno what do you think? |
first of all, great work @Rid! check my comments below :)
Probably I should have worked on this on a specific branch instead of doing it straight into
I wonder why FIG does not have a PSR-15-like for HTTP Clients yet. Could this help us sort out this problem or would it just add to its complexity?
One alternative is to replace client discovery (php-http/discovery or psr-discovery/discovery) with a custom discovery factory that sets up the client according to our needs. WDYT?
I'd rather replace
Cool! I thought this was what
Great catch!
Awesome addition! The current DX is quite poor IMHO, this is a important addition to the right direction!
My objective for this package is that it would be 100% independent on any sort of implementation and just rely on PSR contracts, so the end users could throw in their own preferred implementation for each contract. I think we should open a new branch where we start working towards that. |
I think it's a good idea, I wonder if there's any standard way to discover the unix socket connection options? Or would this have to be hard-coded dependent on the implementation?
I find that
I agree that it should be the aim, it a shame that TCP/Unix socket transport is not part of the PSR-18 spec, it would have made it a lot easier! |
We recently made some changes to https://github.com/beluga-php/docker-php/blob/master/src/DockerClientFactory.php based on beluga-php/docker-php-api#14
There's a few issues around the return types, and discovery classes need to be updated to psr17/18.
However the main issue is around sockets, specifically Guzzle only supports sockets via curl opts, for example guzzle/guzzle#1962. However because we're using
PluginClient
there's no way to pass the options to guzzle.In symfony/http-client it uses the
bindto
option to bind to a unix socket using the same curl_opt (https://github.com/symfony/http-client-contracts/blob/main/HttpClientInterface.php).Unfortunately this makes Unix socket communication implementation specific and not something which could be supported using
PluginClient
.Therefore we need to make a decision of whether to move back to php-http/socket-client or make our library slightly more opinionated and use a specific http client.
socket-client
is going to be abandoned shortly, however forcing a http client is also not ideal as it may conflict with an existing client, or the user may have a different preference.Taking everything into account, I think switching back to
socket-client
and either making a PR to fix the issues, or creating an issue with the maintainer there is probably the best way forward right now. However I'd definitely like to hear what others think?@flavioheleno
The text was updated successfully, but these errors were encountered: