search

keenlabs / KeenClient-PHP

Official PHP client for the Keen IO API. Build analytics features directly into your PHP apps.
https://keen.io/docs
MIT License
133 stars 57 forks source link

Update to Guzzle 4.x for HHVM support #64

Closed seeekr closed 7 years ago

seeekr commented 9 years ago

Is anyone else using HHVM and wishing that the Keen IO PHP client would work on top of that? I'm not sure what else needs to be done, but Guzzle could be updated to 4.x (or even 5.x) and that would be one step towards supporting HHVM. Are there any plans or work already underway?

joshed-io commented 9 years ago

Thanks for opening the issue.

I cross-posted the issue on Keen's developer group to get some discussion flowing :)

seeekr commented 9 years ago

Awesome, thanks! I think overall this shouldn't be too big of a deal to work out / implement since HHVM has progressed really well in terms of supporting all kinds of frameworks etc. Would be lovely to get discussion flowing indeed! :)

joshed-io commented 9 years ago

Sweet! @kmfk whatcha think?

k-k commented 9 years ago

We're not on HHVM - but I don't mind helping to make the move. There's some cleanup on the service description that could be done at the same time that allows for better/easier validation that Ive been meaning to go back and handle.

I've used Guzzle 4 in few clients now and has worked great. But the guzzle-services and underlying command components that were done for guzzle v4+ are still pre 1.0.

Lastly - Guzzle 5 is also out - but I havent had a chance to play around with it yet.

k-k commented 9 years ago

@seeekr - interesting. I get a green test from travis on HHVM with the current code. What errors do you run into?

seeekr commented 9 years ago

@kmfk thanks for the feedback. Well, it has actually been a while ago that I had attempted to integrate Keen.IO into that particular project of mine that runs on HHVM, so I can't remember what the error message was. I definitely remember doing my research afterwards and getting to understand that Guzzle 3.x was not compatible with HHVM and that version 4.x specifically addressed those incompatibilities. What I'll do is have another go at trying the client on HHVM as soon as I have a bit of time for that and then I'll report back. Until then I'll leave you with a link to (one of the?) thread(s) talking about Guzzle and HHVM: https://github.com/guzzle/guzzle/issues/538#issuecomment-36467859 -- It seems that there was lots of work from the HHVM side done on compatibility between the two, so it may be that the Facebook guys were able to fix HHVM during the past 2 or so months to work with Guzzle 3 with no issues (or not relevant to us) any more left.

joshuataylor commented 9 years ago

Guzzle 5.x will also require 5.4.0, allow_url_fopen to be enabled, and recent cURL (if using cURL).

http://docs.guzzlephp.org/en/latest/overview.html#installation

simpleigh commented 9 years ago

Guzzle 4.x also requires PHP 5.4 (https://github.com/guzzle/guzzle/blob/4.0.0/composer.json). It might be worth thinking a little more about this, as PHP 5.3 is still the default version provided with Ubuntu 12.04 (http://packages.ubuntu.com/precise/php5) which is only two years old and supported until April 2017. Ubuntu 14.04 (the next LTS version) includes PHP 5.5 but is only 8 months old. I'm certainly still running lots of Ubuntu 12.04 servers, and suspect that many others are too.

I'm absolutely not suggesting delaying adoption of future Guzzle versions for years, but would perhaps argue that doing so now might be too soon.

Does anyone have any statistics on PHP 5.4 adoption? Is it difficult to support Guzzle 3.x and 4.x simultaneously (either within the same codebase or by maintaining separate versions)?

simpleigh commented 9 years ago

@joshuataylor from your link I don't think you need both allow_url_fopen and recent cURL - one should do. Also it's not really a "recent" cURL version: 7.16.2 was released on Apr 11 2007 (http://curl.haxx.se/docs/releases.html) and has been available in many distributions (at least the ones I've checked) for years.

joshuataylor commented 9 years ago

@simpleigh Drupal 8 requires PHP 5.4 (https://www.drupal.org/requirements), symfony2 is 5.3 (http://symfony.com/doc/current/reference/requirements.html), Zend Framework2 is 5.3 (http://framework.zend.com/about/faq/), Laravel requires 5.4 (http://laravel.com/docs/4.2/installation).

nicklee1990 commented 9 years ago

just checking in on this issue, are there any plans to update and move to a more recent version of guzzle i.e. 5.* or 6.*?

louiszuckerman commented 8 years ago

Any thoughts on updating to guzzle 6?

caseysoftware commented 8 years ago

FYI: I looked into upgrading to Guzzle 6 recently.

The Service Description class has been removed from core and split into Guzzle Service and the Guzzle Description Loader, neither of which have been updated to Guzzle 6.

I just submitted #85 which bumps the dependencies (technically, unnecessary) so they're compatible with G6. Once that's ready, it should be a (hopefully) painless transplant.

djbusby commented 8 years ago

Guzzle 6.x shouldn't block from using Guzzle 5.x right? Which would get this off the deprecated 3.x series. It's a blocker for me

josephwegner commented 7 years ago

I am going to close all guzzle version related issues, and create a new central issue for getting on the latest guzzle. I will link here for reference.