-
Notifications
You must be signed in to change notification settings - Fork 26
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: exponential backoff on retrieval failure #63
feat: exponential backoff on retrieval failure #63
Conversation
What's newWhen retrieving the comments, if there is any failure, the script will attempt again to fetch the comments. In the meantime, it updates the comment in the Issue saying that it is retrying, until eventually put a final error message if it keeps failing. The amount of tries and the delay is configurable through variables within the configuration file. Related tests have been added as well. QA runMeniole#6 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good overall but consider changing the property names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps in the future we can parse time the same way we do with our other plugins like the follow up and unassignment with the plain english strings "3.5 days" "7 days"
@0x4007 We can do that although in this case it cannot be more than 6 hours before the Action is shut down, don't know if it is that useful. I guess we can still put "10 minutes" or "1 hour". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine
6d8bd23
into
ubiquity-os-marketplace:development
actually there is an official octokit plugin plugin-retry that handles retries for all octokit requests - we are already using this in the kernel |
@whilefoo thanks for the information. I can switch to that package within another pull request. |
Resolves #4