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

[GOBBLIN-2110]Made retry_exception_predicate configurable in RetryerFactory #4001

Conversation

pratapaditya04
Copy link
Contributor

@pratapaditya04 pratapaditya04 commented Jul 15, 2024

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots (if applicable):
    RetryerFactory does not give option to set retry_exception_predicate.Made it configurable

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2024

Codecov Report

Attention: Patch coverage is 41.93548% with 18 lines in your changes missing coverage. Please review.

Project coverage is 47.05%. Comparing base (4721d08) to head (c581fff).
Report is 7 commits behind head on master.

Files Patch % Lines
.../org/apache/gobblin/util/retry/RetryerFactory.java 41.93% 16 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4001      +/-   ##
============================================
+ Coverage     45.48%   47.05%   +1.56%     
- Complexity     3222     8072    +4850     
============================================
  Files           705     1553     +848     
  Lines         27558    61574   +34016     
  Branches       2777     6977    +4200     
============================================
+ Hits          12535    28973   +16438     
- Misses        13960    29925   +15965     
- Partials       1063     2676    +1613     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pratapaditya04 pratapaditya04 force-pushed the configurable_retry_exception_predicate branch from 95fc419 to d64067f Compare July 17, 2024 07:19
@@ -1061,6 +1061,8 @@ public class ConfigurationKeys {
// describes a comma separated list of non transient errors that may come in a gobblin job
// e.g. "invalid_grant,CredentialStoreException"
public static final String GOBBLIN_NON_TRANSIENT_ERRORS = "gobblin.errorMessages.nonTransientErrors";
// Key to store a comma-separated list of exception class names that should be retried
public static final String EXCEPTION_LIST_FOR_RETRY_CONFIG_KEY = "EXCEPTION_LIST_FOR_RETRY";
Copy link
Contributor

Choose a reason for hiding this comment

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

approving for now, but lets change the RHS to small case in the next commit and future prs

@arjun4084346 arjun4084346 merged commit 55fdecb into apache:master Jul 17, 2024
6 checks passed
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.

3 participants