Reject events larger than the max size allowed #11623
Conversation
There was a problem hiding this comment.
Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
Hey @xocasdashdash great addition thanks.
About testing: we record snapshots when running the test against the AWS cloud and then compare the snapshot against the result from Localstacak - but I can create the initial snapshot for you once the other things are resolved :) |
|
👋
|
|
@xocasdashdash - hey I fixed the test created the snapshot and fixed the validation - since AWS throws a validation error - this is not always so easy to guess how AWS deals with validation - this is why the snapshots help a lot. |
Ahh fantastic, yeah i thought about this and had some local code working in the same way. It seems that some specific labels need to be added. Could you add them? |
8168879 to
2e519d0
Compare
2e519d0 to
bf2fc7a
Compare
| @@ -199,6 +199,8 @@ def validate_event(event: PutEventsRequestEntry) -> None | PutEventsResultEntry: | |||
| "ErrorCode": "InvalidArgument", | |||
| "ErrorMessage": "Parameter Detail is not valid. Reason: Detail is a required argument.", | |||
| } | |||
| elif event.get("Detail") and len(event["Detail"]) >= 262144: | |||
There was a problem hiding this comment.
We can assume that the detail exists here of it would have gone through line 197
| elif event.get("Detail") and len(event["Detail"]) >= 262144: | |
| elif len(event["Detail"]) >= 262144: |
|
@joe4dev Hi! Sorry for the tag, i've just merged master into the branch and I believe this is ready to be merged. Can you take a look? not sure if there's any next steps i should be following |
There was a problem hiding this comment.
@xocasdashdash Great addition - all the tests are green now - feel free to merge :)
hey! i'm not able to! but i'd love to see this change merged! :D |
Motivation
To close #11622
Changes
Adds a new check on the validation of events sent to an event bus.
TODO
What's left to do: