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

feat(batch): add option to not throw FullBatchFailureError when the entire batch fails #2711

Merged

Conversation

arnabrahman
Copy link
Contributor

Summary

Currently, when using the Batch Processor utility, if all the records in a batch are marked as failed, the utility throws a FullBatchFailureError. However, there can be scenarios where we do not want to throw an error for this condition.

This PR adds a throwOnFullBatchFailure option, giving users the ability to bypass the FullBatchFailureError when the entire batch fails.

Changes

  • Introduce throwOnFullBatchFailure option that can be passed as an option inside processPartialResponse or processPartialResponseSync functions
  • During full batch failure
    • If throwOnFullBatchFailure is false, we will not throw FullBatchFailureError exception
    • If throwOnFullBatchFailure is not provided or is true, we will keep the current implementation of throwing FullBatchFailureError exception
  • Update the relevant doc blocks
  • Unit tests for full batch failure for processPartialResponseandprocessPartialResponseSync` function
  • I believe we have to update the docs but couldn't figure it out in which section. For now, I have added a line for the option.

Issue number: #2122


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@arnabrahman arnabrahman requested review from a team as code owners June 30, 2024 11:40
@boring-cyborg boring-cyborg bot added batch This item relates to the Batch Processing Utility documentation Improvements or additions to documentation tests PRs that add or change tests labels Jun 30, 2024
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Jun 30, 2024
@dreamorosi dreamorosi linked an issue Jun 30, 2024 that may be closed by this pull request
2 tasks
@github-actions github-actions bot added the feature PRs that introduce new features or minor changes label Jun 30, 2024
@leandrodamascena
Copy link
Contributor

Note to remember: @dreamorosi please let's talk before/after merging this PR to see if it makes sense to add the same in Python and @aws-powertools/lambda-dotnet-core

cc @hjgraca

@dreamorosi
Copy link
Contributor

Hi @leandrodamascena, we talked about if a few weeks ago when we did the v3 planning, the consensus was yes, since it impacts the scaling of the poller in some cases.

We can talk about it again also.

@dreamorosi
Copy link
Contributor

Hi @arnabrahman - thank you for the PR!

I'll wait until I can talk with @leandrodamascena tomorrow to start reviewing the PR.

Thank you for your patience!

@dreamorosi dreamorosi added the on-hold This item is on-hold and will be revisited in the future label Jul 1, 2024
@dreamorosi
Copy link
Contributor

Hi @arnabrahman - thank you for the patience.

I'll make a couple of commits to the branch to explain the feature in the documentation before we can merge it.

@dreamorosi dreamorosi removed the on-hold This item is on-hold and will be revisited in the future label Jul 4, 2024
@dreamorosi
Copy link
Contributor

Note to remember: @dreamorosi please let's talk before/after merging this PR to see if it makes sense to add the same in Python and @aws-powertools/lambda-dotnet-core

cc @hjgraca

And to follow up on this: we went through the feature and we are aligned on the fact that we should move forward. We will also add the same flag on the other versions as time allows it.

@leandrodamascena leandrodamascena self-requested a review July 8, 2024 15:05
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Approved! Amazing work @arnabrahman and @dreamorosi!

I'm adding this new feature in Python.

@dreamorosi dreamorosi merged commit 74198ef into aws-powertools:main Jul 9, 2024
11 checks passed
@arnabrahman arnabrahman deleted the 2122-not-throw-full-batch-failure branch September 11, 2024 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch This item relates to the Batch Processing Utility documentation Improvements or additions to documentation feature PRs that introduce new features or minor changes size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: ability to not throw error on full batch failure
4 participants