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

Guzzle 4 and the AWS SDK #311

Closed
jeremeamia opened this issue Jun 16, 2014 · 25 comments
Closed

Guzzle 4 and the AWS SDK #311

jeremeamia opened this issue Jun 16, 2014 · 25 comments

Comments

@jeremeamia
Copy link
Contributor

Since Guzzle 4 was released in March (and even before then), we've received several requests for us to update the AWS SDK for PHP to use Guzzle 4. Earlier this month, we tweeted about it and received some pretty positive feedback about the idea. Now we've blogged about it, too.

We think that updating the SDK to use Guzzle 4 is the best thing for the SDK and SDK users, but we want to hear from you. Do you have any questions or concerns? What other feedback or ideas do you have? Be sure to check out the blog post and leave your feedback here.

@jeremeamia jeremeamia changed the title Guzzle 4 and the AWS SDK for PHP Guzzle 4 and the AWS SDK Jun 16, 2014
@stevenscg
Copy link

This sounds great! We haven't hooked in to the lower-level functionality yet, so I wouldn't expect any significant problems upgrading. PHP 5.4 requirement is fine as well.

@moffe42
Copy link

moffe42 commented Jun 17, 2014

Would be great. The AWS SDK is the final part of our software stack that needs to be upgraded, for us to use Guzzle 4. So please go ahead. But you might need to keep up maintenance on the 2.x branch, until the most of the community keeps up with Guzzle 4.

@frankdejonge
Copy link
Contributor

The only thing I can think which might put people off here is the raised minimum php version, which I don't think is enough of an argument not to do this. PHP 5.3 has been EOL for quite a long time now. People can't expect to have the latest packages on an old php version, that's just how these things go. 👍

@skyzyx
Copy link
Contributor

skyzyx commented Jun 17, 2014

People are still using PHP 5.3.x??? ;)

@frankdejonge
Copy link
Contributor

@skyzyx welcome to the wonderful world of "maintaining open source packages". Yes, there are quite a lot of people that are on 5.3 still. There's got to be some real incentive to make them move forward though, if more maintainers do it, it'll all move a little quicker.

@jeremeamia
Copy link
Contributor Author

Gathering comments from other places:

Comment from Derek R. on the blog post:

Moving to require PHP >= 5.4 is inevitable and appropriate for a 3.0.0 release but I feel like it would require at least maintenance releases be continued for version 2 of the SDK for a time.

Comment from @WyriHaximus on Twitter:

Yeah it looks great! Love to find a way to make my adapter compatible :D!

@jeremeamia
Copy link
Contributor Author

@FrenkyNet Oh, I think @skyzyx knows that world all too well. 😃

@frankdejonge
Copy link
Contributor

@jeremeamia seems like @skyzyx and me must live in a different part of the spectrum then :P. Btw, Flysystem will also be moving to 5.4 in the next release (which will shortly lead to the 1.x). I've asked around about it too, all I got were "go for it" responses.

@WyriHaximus
Copy link

@jeremeamia He's not the only one 😄. Seen stuff run on 5.2 not so long ago. Same goes for old MySQL or VCS software etc etc. @FrenkyNet is right, people are still running old software but they have a working SDK for that version. So we don't have to stay stuck there. I'm very welcome moving on and using Guzzle 4 in the SDK 👍.

@maknz
Copy link

maknz commented Jun 17, 2014

👍 I'm all for being progressive. Plenty of projects are dropping 5.3. Some might get caught out if they're coupled to Guzzle 3.x, but given this would be a major release, BC breaks should be expected.

@mtdowling
Copy link
Member

@maknz Glad to hear that. One of the cool things about Guzzle 4 is that it can be used the same project as Guzzle 3 because it utilizes a different namespace.

@skyzyx
Copy link
Contributor

skyzyx commented Jun 18, 2014

Yeah, I was kidding. But I've been running 5.5 for most of the year now. I make it a point to stay up-to-date and validate my projects against newer versions sooner rather than later.

@stof
Copy link

stof commented Jun 19, 2014

One of the thing to be careful about is that this will be a BC break in the SDK, as your return values are the Guzzle3 models in many places, and upgrading to Guzzle4 would then force to change the API of the SDK (unless you depend on Guzzle4 for the logic and on Guzzle3 at the same time for the returned model, which would be insane).
I think this new major version of the SDK should decouple its public API from Guzzle, to avoid such issues in the future. Low levels APIs would use Guzzle, but the public API of the SDK should rely on its own model classes (or an arrays for collections). This would mean that most users of the SDK would not need to alter their code the next time a Guzzle major version happens.

@mtdowling
Copy link
Member

@stof We do know that this would be a BC break and that it would require a new major version of the SDK. We've talked about decoupling from Guzzle, but it wouldn't be worth it considering we would have to reinvent things that Guzzle was designed to provide: an event system, HTTP messages, HTTP clients, web service clients, event subscribers (and much more). We are planning on moving a few things a bit further from Guzzle though: the return value of operations would be an AWS object, there's an AwsClientInterface, we would use AWS service descriptions directly rather than Guzzle service descriptions, we'd use our own configuration logic rather than Guzzle's, etc.

This would mean that most users of the SDK would not need to alter their code the next time a Guzzle major version happens.

I'm the author of Guzzle, and I can tell you that I have no plans for a new major version in Guzzle anytime in the next few years (maybe even ever). If there ever was a major new version, it would not be a rewrite but rather a few small breaking changes that were needed to solve a bug or security issue. If that did happen, it would likely have no impact on the SDK.

@fruitl00p
Copy link

Back on upgrading SDK -> Guzzle4: Upping the minimal version to 5.4+ seems reasonable if the current version would still receive some maintenance releases. Other than that: 👍

@jeremeamia
Copy link
Contributor Author

From dawalama on the blog comments:

Thank you for detailed analysis. Guzzle 4 really does have nice improvements. Improved performance and better async requests handling are sufficient reasons for the update in my opinion. I vote for the update.

A new major version for SDK would be really welcomed.

@bakura10
Copy link
Contributor

bakura10 commented Jul 1, 2014

I have also received a lot of requests from updating to Guzzle 4 for a lot of integrations I've made, so having everything unified into Guzzle 4 is something I'd like to have.

Minimal version of 5.4 does not bother me! Big +1 from me

@dwenaus
Copy link

dwenaus commented Jul 14, 2014

please upgrade to Guzzle 4

@cdnsteve
Copy link

cdnsteve commented Aug 6, 2014

From my perspective, we're currently using PHP 5.3 due to our hosting providers setup at Acquia. They do support PHP 5.5 so we're eventually looking to migrate to that but PHP 5.4 isn't an option.
If the upstream has a new release we should support that too.

@GrahamCampbell
Copy link
Contributor

@cdnsteve. If you want have 5.3 and upwards use the current aws sdk. If you have php 5.4 and upwards, you'll be able to use the new aws sdk (when it comes out). It certainly doesn't require php 5.4.x - that would be absurd - it only requires you to be using at least php 5.4. Both the current, and upcoming aws sdks will run fine on php 5.5.

@Sazpaimon
Copy link
Contributor

Since PHP 5.3 is actually now EOL (see http://php.net/eol.php) it makes sense to bump up the minimum version of the SDK to 5.4 anyway.

@gondo
Copy link

gondo commented Sep 9, 2014

+1

@GrahamCampbell
Copy link
Contributor

@mtdowling Will this be guzzle 5 now then?

@iby
Copy link

iby commented Sep 15, 2014

+1

@jeremeamia
Copy link
Contributor Author

Thanks for your input everyone!

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

No branches or pull requests