-
-
Notifications
You must be signed in to change notification settings - Fork 103
Feature: Added S3ClientProvider to AWSS3StorageCache and AWSS3StorageImageProvider #362
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
Feature: Added S3ClientProvider to AWSS3StorageCache and AWSS3StorageImageProvider #362
Conversation
S3CannedACL acl) | ||
=> AsyncHelper.RunSync(() => CreateIfNotExistsAsync(options, acl)); | ||
S3CannedACL acl, | ||
IServiceProvider serviceProvider) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is a breaking change, because it requires users of this method to add the serviceProvider param.
namespace SixLabors.ImageSharp.Web; | ||
|
||
/// <summary> | ||
/// Provides a common interface for AWS S3 Bucket Client Options. | ||
/// </summary> | ||
internal interface IAWSS3BucketClientOptions | ||
public interface IAWSS3BucketClientOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to make the internal interface public here, because it is used as a param of the S3ClientProvider.
@marklagendijk We've got failing tests here. Could you please have a look? |
I want to avoid breaking changes here or else it's major version update. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
===================================
- Coverage 85% 85% -1%
===================================
Files 82 82
Lines 2355 2361 +6
Branches 354 356 +2
===================================
+ Hits 2019 2023 +4
Misses 232 232
- Partials 104 106 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this and sorry about the delay. I had to wait until I could make a breaking change.
I'll merge and make any required changes following a local review.
Prerequisites
Description
Implementation of #360 for AWS.