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

Implementation branch #176

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

ekaschaw
Copy link

@ekaschaw ekaschaw commented Aug 4, 2023

This PR handles the implementation of a few aspects of the RetryHandlerSkeleton. Specifically, Additive Increase Multiplicative Decrease Batch scaling along with the basic case of executing retries on a detected Retry-able Exception.

There are two test suites in this PR:

  1. AIMD unit testing: I created simple test cases that ensure when a task is completed successfully or fails, the batch size is reflected correctly and is configurable to the client specifications.
  2. Retry Handling and Exception Wrapping: The test simulates a list of 10 Tasks of which 3 throw a retryable exception that the handler recognizes and enqueues for retry. The result is the list of results for the successful tasks and a second list of task_IDs for tasks that either 1. Exceeded the retry cap (Default cap = 3 retries) or 2. Non-retryable in nature.

Copy link
Collaborator

@raghumdani raghumdani left a comment

Choose a reason for hiding this comment

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

On a very high level, please ensure the linter are passing, write unit tests for each class and document the testing steps in the description of this PR.

Copy link
Collaborator

@raghumdani raghumdani left a comment

Choose a reason for hiding this comment

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

I think implementation still requires some cosmetic and logical fixes.

@raghumdani
Copy link
Collaborator

The tests are failing. Please check.

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.

2 participants