-
-
Notifications
You must be signed in to change notification settings - Fork 103
Updated AWSSDK.S3 to 4.0 #385
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
Conversation
The 3.x range is also still being updated but I think we can upgrade to the 4.x range because we don't need net3.5 support |
@@ -9,6 +9,8 @@ namespace SixLabors.ImageSharp.Web; | |||
|
|||
internal static class AmazonS3ClientFactory | |||
{ | |||
static AmazonS3ClientFactory() => AWSConfigs.InitializeCollections = true; |
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.
Altering global behavior for transitive dependency does not appear be a prudent. It would be more appropriate to update the code to accommodate changes in behavior.
Thanks, I'm looking into it a bit. Managed to get the test failing locally, I'll try to take some time to investigate that soon! |
I had a look myself and there's something odd going on. The test is failing as there seems to be an additional 43 bytes of data attached (I'm assuming at the beginning of the stream) when we retrieve the file from the cache. I don't know yet whether this is an issue with the |
@mdupras to follow up I discovered that there was a behavioral change in AWS. It's inserting a byte sequence before our output.
I added the following locally and everything works!!
I've opened PR #392 to replace this one as I updated Azure also |
Thanks @JimBobSquarePants ! The other annoying thing with the new version is if a static variable isn't changed, all the collections types in responses will be null instead of empty. Great way of sending nullref everywhere. |
Prerequisites
Description
Updated the AWSSDK.S3 to 4.0 release and fix the compilation issue.
Now
metadata.LastModified
can be null, I don't think it's like to happen in this scenario, but if it does the default Date is Now to assume the Image needs to be refreshed.