Skip to content
This repository was archived by the owner on Oct 26, 2019. It is now read-only.

Conversation

@krzaczek
Copy link
Contributor

Guzzle library upgrade to 5.3

This is a PR to upgrade guzzle library form version 4.x to 5.3. I did this because ver 4 of guzzle conflicts with lots of new libraries like aws/aws-sdk-php

Decided to pick Guzzle 5.3 since it supports PHP 5.4 and I didn't want to make any BC

Please review the code and let me know what You think of it :)

@krzaczek krzaczek changed the title [WIP] Guzzle 5.x support Guzzle 5.x support Nov 20, 2015
@ubermuda
Copy link
Contributor

Hi @krzaczek and thanks for your amazing work! That is a huge PR and reviewing it will take some time, but we'll get there! Don't worry if you don't get an answer in the next few days, we'll make sure to get back to you in due time :)

@krzaczek
Copy link
Contributor Author

Hi @ubermuda ... cool ... I'm waiting for comments :)

@sroze
Copy link
Contributor

sroze commented Nov 29, 2015

👍

@krzaczek
Copy link
Contributor Author

@ubermuda I have added better exceptions handling that is compatible with current version. All exceptions are thrown as Docker\Exception\APIException

@joelwurtz
Copy link
Member

Hi @krzaczek, first of all thanks for this PR.

However our goal for the http client is to use this library : https://github.com/php-http/httplug so there is no conflict / duplication if someone use guzzle 5, or guzzle 6 or zend/http or .... (you see my point)

Is it too much to ask to rework this PR by using this library ?

This is my mind for a long time (abstraction of http client) as it's real pain actually, so it will be a huge win to have this.

@krzaczek
Copy link
Contributor Author

Well I need this .. as Guzzle 4.x conflicts with aws-sdk -- so sure ... but I don't know httpplug .. so this my take some time ... You ma close this PR ... i'll start a new one when I'm ready

@krzaczek
Copy link
Contributor Author

I see it's "early" development - alpha - hmmmm

@joelwurtz
Copy link
Member

I'm one the maintainer and our goal is to have something stable in the beginning of 2016, in fact i start to work on this for docker-php (and also other libraries)

Let's keep this open for the moment as a reminder

@krzaczek krzaczek changed the title Guzzle 5.x support [WIP] Guzzle 5.x support Dec 11, 2015
@dhopkalo
Copy link

I think it will be best in the future use this https://github.com/php-http/httplug

But now this upgrade required, cause guzzle 4 now is very rare version.

@joelwurtz
Copy link
Member

@krzaczek I add time to finally handle the refactoring of the library, there is now a 1.x branch which should be the stable release and maintained with docker remote api update.

It does not require guzzle anymore, so if you want to give it a try (be aware that it's completly BC from the actual documentation / library)

@krzaczek
Copy link
Contributor Author

krzaczek commented Jan 4, 2016

Yes I know .. i'll give it a try tommorow

pawel (at) mobile

On 4 sty 2016, at 18:17, Joel Wurtz notifications@github.com wrote:

@krzaczek I add time to finally handle the refactoring of the library, there is now a 1.x branch which should be the stable release and maintained with docker remote api update.

It does not require guzzle anymore, so if you want to give it a try (be aware that it's completly BC from the actual documentation / library)


Reply to this email directly or view it on GitHub.

@krzaczek
Copy link
Contributor Author

krzaczek commented Jan 5, 2016

@joelwurtz Works perfectly :) We can close this PR now.

@joelwurtz
Copy link
Member

Wondering also, will you be interested if it doesn't define a specific client implementation (so you can use guzzle instead of the socket client ?).

Counterpart, some calls and features will not work with all clients implementation, like connecting to a unix socket domain, ssl or real time feature.

@krzaczek
Copy link
Contributor Author

krzaczek commented Jan 5, 2016

Well like You said .. it won't 100% work on any other client .. so I don't think You should allow it. Since socket support is compiled in PHP by default what's the point ? :)

@krzaczek
Copy link
Contributor Author

krzaczek commented Jan 5, 2016

Ps. I do have a bug but in https://github.com/php-http/message/blob/master/src/Encoding/FilteredStream.php#L118

$buf = $this->read(1048576);

So a code

$body = $logs->getBody()->getContents();

Fails with an exception
Fatal error: Maximum function nesting level of '250' reached, aborting! in /opt/web/gitlab/printu/printu/vendor/clue/stream-filter/src/CallbackFilter.php on line 62

while(!$body->eof()) {
    $log .= $body->read(4096);
}

I Can read this stream manually ... and then it works

@joelwurtz
Copy link
Member

Yeah it's not really a bug as it's trying to read the full message, but since you can have chunked message, it's possible to have more than 256 calls to the read function, i will lower the buf size to 8192, as afterall it does not make any sense to have less or more in php if nothing is setted (php already have a internal buffer of 8192 bytes)

@joelwurtz
Copy link
Member

So anyway really thanks for the PR even if it didn't make to the master, it may have push me to move thing faster :).

@joelwurtz joelwurtz closed this Jan 8, 2016
@krzaczek krzaczek deleted the guzzle5 branch January 28, 2016 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants