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

[WIP] V3 #23

Closed
wants to merge 3 commits into from
Closed

[WIP] V3 #23

wants to merge 3 commits into from

Conversation

bakura10
Copy link
Contributor

This oversedes #22. Sorry I didn't have more time today I'll try to do that tomorrow. This one will also include switch to PSR-4 and min dependency of 5.5

@bakura10
Copy link
Contributor Author

Hi @jeremeamia

I've made progress on it. Here are the few things I've done:

  • Switch to PSR-4
  • Use ::class and short array notation
  • In CloudFront and S3 view helpers, I have removed the possibility to generate HTTP or relative link (the HTTP was deprecated and I think we really should enforce HTTPS anyway).
  • Exception no longer extend a common interface because new SDK does not have one
  • In current version, you added several things to the User Agent of all requests made with the SDK. I've removed that because I have no idea about how we can do it now.

There are a few things I'm a bit unsure, and one test in S3 is failing and I'm not sure how to solve it. From now on I think you can use this version and finish the work :).

Have a nice day!

@bakura10
Copy link
Contributor Author

I also realized that most S3 tests were failing because the generated URLs were completely different (see the diff in tests). I'm not sure if those are a new style of URL in S3.

'aws_zf2' => [
'session' => [
'save_handler' => [
'dynamodb' => [
// Locking strategy used for doing session locking.
// 'locking_strategy' => null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 'locking' => false, now

@jeremeamia
Copy link
Contributor

Looking good. Thanks @bakura10!

I also realized that most S3 tests were failing because the generated URLs were completely different (see the diff in tests). I'm not sure if those are a new style of URL in S3.

We have moved to a more regionalized and secure S3 experience for V3. This means that region must be specified when creating an S3 client, Signature Version 4 is used exclusively for signing requests, and we no longer move bucket names into the host name.

@bakura10
Copy link
Contributor Author

@jeremamia, I've done the various changes, and changed the namespace to AwsModule. I let you know have a look at it, and fix the missing test for S3 ;).

Have a nice day!

@jeremeamia
Copy link
Contributor

@bakura10 Thanks!

@bakura10
Copy link
Contributor Author

Is there anything else I can do @jeremeamia ? :)

@jeremeamia
Copy link
Contributor

jeremeamia commented Jun 12, 2015 via email

@jeremeamia
Copy link
Contributor

Closing in favor of #27, which includes these changes.

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

Successfully merging this pull request may close these issues.

2 participants