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

Fixed retry and duplicated replacements #3015

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

Conversation

Seetaramayya
Copy link
Contributor

@Seetaramayya Seetaramayya commented Mar 20, 2023

For good amount of time our scala-steward jobs are failing. Following changes fixed in our case, so creating the PR, might be useful port the changes to the main repo.

I hope this PR address the issue #2934

New Retry logic

  • If Retry-After header present, then use the header value to retry (this was there before)
  • otherwise retry request after time specified by X-Ratelimit-Reset header. Header value is UTC epoch seconds. Here I took some assumptions
    • if the difference between header value and current UTC epoch seconds is more than 60 seconds then I retry after 60 seconds. That means max wait time would be 60 seconds.
    • if the difference is negative then I retry after 1 second. That means min wait time is 1 second
  • if neither of the headers present, wait 1.second

Here is the github suggested best-practices

image

Another issue

If there are duplicated replacements then the job was failing with StringIndexOutOfBoundsException. Added unit test to verify the behaviour.

@fthomas
Copy link
Member

fthomas commented Mar 20, 2023

Another issue

This deserves its own PR. I would also prefer to have a test for that in RewriteTest instead of in EditAlgTest.

@Seetaramayya
Copy link
Contributor Author

Another PR crated to address the other issue #3016

@Seetaramayya
Copy link
Contributor Author

Are you going to merge this PR?

@odisseus
Copy link

In order to make this PR mergeable, you must resolve the conflicts first.

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