Create GithubRetry.DEFAULT_* constants#3006
Conversation
| from github.Requester import Requester | ||
|
|
||
| DEFAULT_SECONDARY_RATE_WAIT: int = 60 | ||
| DEFAULT_STATUS_FORCELIST = list(range(500, 600)) |
There was a problem hiding this comment.
I think we should also add 403 to the default.
| DEFAULT_STATUS_FORCELIST = list(range(500, 600)) | |
| DEFAULT_STATUS_FORCELIST = list(range(500, 600)) + [403] |
There was a problem hiding this comment.
@jodelasur Any objections? Otherwise I would apply these changes.
There was a problem hiding this comment.
I guess this is OK. See my comment below though. May we could apply this and not the other one?
| # to determine if we really retry that 403 | ||
| kwargs["status_forcelist"] = kwargs.get("status_forcelist", list(range(500, 600))) + [403] | ||
| kwargs["allowed_methods"] = kwargs.get("allowed_methods", Retry.DEFAULT_ALLOWED_METHODS.union({"GET", "POST"})) | ||
| kwargs["status_forcelist"] = kwargs.get("status_forcelist", DEFAULT_STATUS_FORCELIST) + [403] |
There was a problem hiding this comment.
| kwargs["status_forcelist"] = kwargs.get("status_forcelist", DEFAULT_STATUS_FORCELIST) + [403] | |
| kwargs["status_forcelist"] = kwargs.get("status_forcelist", DEFAULT_STATUS_FORCELIST) |
There was a problem hiding this comment.
Given a user provides a custom status_forcelist value without 403, the current logic will still add 403 on init. If we apply this change, this won't happen anymore and 403 won't be in the list. This could affect the custom 403 logic applied for GithubRetry. What do you think?
If you feel this is a non-issue, please go ahead and apply. I'll be offline for a week or so, and won't be able to check on this.
2 participants
Resolves #2970.