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

Closed
wants to merge 1 commit into from
Closed

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 :)

Copy link

Hi there! Unfortunately, this PR has not seen any activity for at least 90 days. If the PR is still relevant to the latest version of Zappa, please comment within the next 10 days if you wish to keep it open. Otherwise, it will be automatically closed.

@github-actions github-actions bot added the no-activity [Bot] Closing soon if no new activity label Jul 24, 2024
Copy link

github-actions bot commented Aug 3, 2024

Hi there! Unfortunately, this PR was automatically closed as it had not seen any activity in at least 100 days. If the PR is still relevant to the latest version of Zappa, please open a new PR.

@github-actions github-actions bot added the auto-closed [Bot] Closed, details in comments label Aug 3, 2024
@github-actions github-actions bot closed this Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-closed [Bot] Closed, details in comments no-activity [Bot] Closing soon if no new activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants