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.

Sign up for GitHub

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

Updated S3.Event #195

Merged

tomerd merged 18 commits into swift-server:main from mufumade:s3_Event_Incorrect

Apr 15, 2021

Contributor

mufumade commented Mar 13, 2021

Motivation:

Modifications:

Changed the S3.Event object size field to an optional due to the missing field on a s3:ObjectRemoved:* event
Added one additional test to check propper json decoding

Result:

s3 event json will now decode successfully even if aws does not populate the size field.


Changed the S3.Event object size field to an optional due to the miss…

b70db6f

…ing field on a s3:ObjectRemoved:* event

Added one additional test to check propper json decoding

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

5 similar comments

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 13, 2021

Can one of the admins verify this patch?

fabianfett reviewed

View reviewed changes

Member

fabianfett left a comment

Thanks for going ahead! Two small suggestions.

Sources/AWSLambdaEvents/S3.swift Outdated Show resolved Hide resolved

Tests/AWSLambdaEventsTests/S3Tests.swift Show resolved Hide resolved

mufumade and others added 2 commits

March 13, 2021 15:25


Update Sources/AWSLambdaEvents/S3.swift

db21f9b

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Tests/AWSLambdaEventsTests/S3Tests.swift

1198d0c

Co-authored-by: Fabian Fett <fabianfett@apple.com>

Contributor Author

mufumade commented Mar 13, 2021

@fabianfett Thanks for the feedback!

Member

fabianfett commented Mar 13, 2021

@swift-server-bot test this please

Member

fabianfett commented Mar 14, 2021

@mufumade Would you mind running swiftformat . once in the project folder, to please the soundness check?

mufumade added 3 commits

March 14, 2021 15:52


swiftformat changes

9704f7d


Merge branch 's3_Event_Incorrect' of https://github.com/mufumade/swif…

b591ae5

…t-aws-lambda-runtime into s3_Event_Incorrect


swiftformat changes

0da8bc4

Contributor Author

mufumade commented Mar 14, 2021

So this was a bit strange. At first swiftformat changed like 30 files although I only changed 2. On the second attempt swiftformat changed only the comment you provided with your suggestions. I hope it's all fine now.

Member

fabianfett commented Mar 14, 2021

@mufumade oh. sorry. it looks to me like our swiftformat is a little out of date. I'm updating it right now and will post another pr. Once that swiftformat pr has landed, we update this pr and run swiftformat again, which then should only affect your changed files. Sorry for the hassle and confusion.

Member

fabianfett commented Mar 14, 2021

For reference: #196

tomerd assigned fabianfett

Member

fabianfett commented Mar 27, 2021

@mufumade Would you mind resolving the merge conflicts and running swiftformat again? After that we should be ready to merge.

Contributor

tomerd commented Mar 28, 2021

@swift-server-bot test this please


Resolved merge conflicts and swiftformat

9cba283

Contributor Author

mufumade commented Mar 29, 2021

@fabianfett have a look at the newest commit.

Member

fabianfett commented Mar 29, 2021

@swift-server-bot test this please

tomerd added this to the 0.5.0 milestone

fabianfett approved these changes

View reviewed changes

Member

fabianfett left a comment

LGTM. Some internal acls have been removed. Do we want this? Should we disable redundantextensionacl? @tomerd wdyt?

Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift Outdated Show resolved Hide resolved

fabianfett reviewed

View reviewed changes

Member

fabianfett left a comment

@mufumade I'm so sorry for all this swiftformat . confusion. Would you mind merging the current main branch and re-adding the dropped internal acls? I hope this is the last time...


Merge branch 'main' into s3_Event_Incorrect

eeda733

Member

fabianfett commented Apr 14, 2021

@mufumade 👋 Ping: Are you still interested in getting this merged? I would love to see this land...

Contributor Author

mufumade commented Apr 14, 2021

Oh. Sorry for the delay. I'll get started right away.

Contributor Author

mufumade commented Apr 14, 2021

@fabianfett So i merged the current main into my branch, ran swiftformat again to check but I am not quite sure what you mean by re-adding the dropped internal acls?

fabianfett reviewed

View reviewed changes

Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift Outdated Show resolved Hide resolved

fabianfett reviewed

View reviewed changes

Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift Outdated Show resolved Hide resolved

fabianfett reviewed

View reviewed changes

Member

fabianfett left a comment

@mufumade I have added suggestions to re-add all the internal ACL that were dropped by swiftformat before. Just add those and we should be ready to go :) Thanks! And sorry for all the confusion.

Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift Outdated Show resolved Hide resolved

Sources/AWSLambdaRuntimeCore/Utils.swift Outdated Show resolved Hide resolved

Sources/AWSLambdaRuntimeCore/LambdaRunner.swift Outdated Show resolved Hide resolved

Sources/AWSLambdaRuntimeCore/LambdaConfiguration.swift Outdated Show resolved Hide resolved

Sources/AWSLambdaRuntimeCore/Lambda+LocalServer.swift Outdated Show resolved Hide resolved

Sources/AWSLambdaEvents/Utils/HTTP.swift Outdated Show resolved Hide resolved

mufumade and others added 9 commits

April 15, 2021 12:43


Update Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift

2c7e127

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/Utils.swift

3e2d491

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift

37b431f

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift

9d82c82

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/LambdaRunner.swift

43882f6

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/LambdaConfiguration.swift

0458ef5

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaRuntimeCore/Lambda+LocalServer.swift

528a2a0

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Update Sources/AWSLambdaEvents/Utils/HTTP.swift

Co-authored-by: Fabian Fett <fabianfett@apple.com>


Merge branch 'main' into s3_Event_Incorrect

d892b50

Member

fabianfett commented Apr 15, 2021

@swift-server-bot test this please


Update Sources/AWSLambdaRuntimeCore/LambdaRuntimeClient.swift

0a676c2

Member

fabianfett commented Apr 15, 2021

@swift-server-bot test this please

tomerd approved these changes

View reviewed changes

tomerd merged commit edbfa79 into swift-server:main

mufumade deleted the s3_Event_Incorrect branch

April 15, 2021 18:07

tomerd mentioned this pull request

AWSLambdaEvents.S3.Event incorrect if event is ObjectRemoved #194

Closed

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