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

Fix support for "aws/aws-sdk-php:^3.0" #1814

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

phansys
Copy link
Member

@phansys phansys commented Sep 19, 2020

Subject

Fix support for "aws/aws-sdk-php:^3.0".

I am targeting this branch, because these changes respect BC.

Related to #1018.

Changelog

### Fixed
- Fixed calls to deprecated method `AwsClient::factory()` when using aws/aws-sdk-php:^3.0;
- Fixed passing required arguments to "sonata.media.cdn.cloudfront" service when using aws/aws-sdk-php:^3.0;
- Fixed respecting "sonata_media.filesystem.s3.region", "sonata_media.filesystem.s3.version" and "sonata_media.filesystem.s3.endpoint" configuration nodes when using aws/aws-sdk-php:^2.0.

### Deprecated
- Deprecated "sonata_media.filesystem.s3.sdk_version" configuration node.

To do

  • Deprecate sonata_media.filesystem.s3.sdk_version configuration node, since there is no way to install "aws/aws-sdk-php" 2.x and 3.x at the same time (this probably was added to switch between version 1.x and 2.x, since 1.x was provided by a different package).
  • Add new CloudFront configuration options required in v3.x (region, version, etc).
  • Update the documentation;
  • Fix the arguments passed to CloudFrontClient::createInvalidation() when using aws/aws-sdk-php:^3.0.

OskarStark
OskarStark previously approved these changes Sep 19, 2020
tests/App/config.yml Outdated Show resolved Hide resolved
@phansys phansys changed the title Allow "aws/aws-sdk-php:^3.0" Fixed support for"aws/aws-sdk-php:^3.0" Sep 21, 2020
@phansys phansys changed the title Fixed support for"aws/aws-sdk-php:^3.0" Fixed support for "aws/aws-sdk-php:^3.0" Sep 21, 2020
@phansys phansys changed the title Fixed support for "aws/aws-sdk-php:^3.0" Fix support for "aws/aws-sdk-php:^3.0" Sep 21, 2020
@phansys phansys marked this pull request as ready for review September 21, 2020 04:43
@phansys phansys requested review from OskarStark and a team September 21, 2020 04:43
jordisala1991
jordisala1991 previously approved these changes Sep 21, 2020
@phansys phansys requested a review from a team September 21, 2020 10:56
/**
* @todo: Make mandatory argument 5 and 6 when support for aws/aws-sdk-php < 3.0 is dropped.
*/
public function __construct(string $path, string $key, string $secret, string $distributionId, ?string $region = null, ?string $version = null)
Copy link
Member

Choose a reason for hiding this comment

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

Please line break this

core23
core23 previously approved these changes Sep 21, 2020
@phansys phansys dismissed stale reviews from core23 and jordisala1991 via b5e0bd8 September 21, 2020 12:46
src/CDN/CloudFront.php Outdated Show resolved Hide resolved
src/CDN/CloudFront.php Show resolved Hide resolved
src/CDN/CloudFront.php Show resolved Hide resolved
src/CDN/CloudFront.php Outdated Show resolved Hide resolved
src/CDN/CloudFront.php Outdated Show resolved Hide resolved
// @todo: Remove the following check and the `else` block when support for aws/aws-sdk-php < 3.0 is dropped.
if (class_exists(Sdk::class)) {
// AWS v3.x.
$arguments['InvalidationBatch'] = $invalidationBatch;
Copy link
Member

Choose a reason for hiding this comment

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

Technicly it is the same, but better see diffrence between both versions.

Suggested change
$arguments['InvalidationBatch'] = $invalidationBatch;
$arguments += ['InvalidationBatch' => $invalidationBatch];

Comment on lines 268 to 280
// @todo: Remove the following check and the `else` block when support for aws/aws-sdk-php < 3.0 is dropped.
if (class_exists(Sdk::class)) {
// AWS v3.x.
$config['credentials'] = [
'key' => $this->region,
'secret' => $this->version,
];

$this->client = new CloudFrontClient($config);
} else {
// AWS v2.x.
$config['key'] = $this->region;
$config['secret'] = $this->version;

$this->client = CloudFrontClient::factory($config);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// @todo: Remove the following check and the `else` block when support for aws/aws-sdk-php < 3.0 is dropped.
if (class_exists(Sdk::class)) {
// AWS v3.x.
$config['credentials'] = [
'key' => $this->region,
'secret' => $this->version,
];
$this->client = new CloudFrontClient($config);
} else {
// AWS v2.x.
$config['key'] = $this->region;
$config['secret'] = $this->version;
$this->client = CloudFrontClient::factory($config);
}
$keysConfig = [
'key' => $this->region,
'secret' => $this->version,
];
// @todo: Remove the following check and the `else` block when support for aws/aws-sdk-php < 3.0 is dropped.
if (class_exists(Sdk::class)) {
// AWS v3.x.
$this->client = new CloudFrontClient($config+['credentials' => $keysConfig]);
} else {
// AWS v2.x.
$this->client = CloudFrontClient::factory($config+$keysConfig);
}

@jordisala1991 jordisala1991 merged commit c185beb into sonata-project:3.x Sep 21, 2020
@jordisala1991
Copy link
Member

Thank you @phansys

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

Successfully merging this pull request may close these issues.

None yet

6 participants