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

extend SQS event handler #1325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

extend SQS event handler #1325

wants to merge 1 commit into from

Conversation

eviltwin
Copy link

@eviltwin eviltwin commented Apr 8, 2024

Description

  • report_batch_item_failures for partial batch processing
  • maximum_concurrency to provide limits to SQS event source scaling
  • remove outdated documentation about FIFO queues being unsupported

@eviltwin
Copy link
Author

eviltwin commented Apr 8, 2024

It was quite a small extension of the existing SQS handler, so I wasn't sure how much documentation would need adding for it... I'm happy to elaborate more on what I put, but the existing options didn't come with much explainer so I wasn't sure if that was a deliberate choice or not.

I corrected the FIFO docs part mostly in passing (I have confirmed that a FIFO queue works just fine).

- report_batch_item_failures for partial batch processing
- maximum_concurrency to provide limits to SQS event source scaling
- remove outdated documentation about FIFO queues being unsupported
Copy link
Collaborator

@monkut monkut left a comment

Choose a reason for hiding this comment

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

It looks like we don't have good test coverage around this, but can you try to add testcases to cover this addition?

It appears we have minor coverage in test_placebo.py has event_source related testcases.

@coveralls
Copy link

Coverage Status

coverage: 74.655% (-0.2%) from 74.81%
when pulling 61f8407 on eviltwin:master
into a38058b on zappa:master.

@eviltwin
Copy link
Author

@monkut thanks, I'd looked for the test coverage when SQS events were originally implemented but I couldn't find them. I'll try to find the time in the next week to add coverage in the places you indicated :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants