Skip to content

Deal with 429s on merge#13

Draft
addshore wants to merge 3 commits intofuzeman:masterfrom
addshore:auto-retry-on-429
Draft

Deal with 429s on merge#13
addshore wants to merge 3 commits intofuzeman:masterfrom
addshore:auto-retry-on-429

Conversation

@addshore
Copy link

@addshore addshore commented Mar 12, 2024

I was getting lots of rate limit messages while trying to merge a bunch
of stuff on my account.

I was getting responses such as:

x-ratelimit: {"name":"AUTHED_API_POST_LIMIT","period":1,"limit":1,"remaining":0,"until":"2024-03-12T15:10:32Z"}
retry-after: 1

I added this retry logic which may be useful, but ultimately it
looks like the rate-limit that is set by the CLI should probably be used,
but is not currently used in the executor for the merge?

I was getting lots of rate limit messages while trying to merge a bunch
of stuff on my account.

I was getting responses such as:
x-ratelimit: {"name":"AUTHED_API_POST_LIMIT","period":1,"limit":1,
"remaining":0,"until":"2024-03-12T15:10:32Z"}
retry-after: 1

I added this retry logic which may be useful, but ultimately it
looks like the rate-limit tha tis set by the CLI should probably be used,
but is not currently used in the executor for the merge?
@addshore
Copy link
Author

I'm happy to rework this to try and make use of the --rate-limit but would appreciate a pointer or 2 as to how @fuzeman would want that approached.
I see that profile currently has _rate_limit(), and so far this is only called by profile.request
The executor ultimately uses Trakt['sync/history'].remove( rather than a general request, hence the rate limit isn't used.

Should I try and just use .request in place of the .remove call? or is there a reason that call is different?

This seems to basiucally fix the issue, and no longer require the
retry code that I wrote in the previous patch
@addshore addshore changed the title Auto retry on 429 in merge Deal with 429s on merge Mar 12, 2024
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.

1 participant