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

Add rate limit error handling #29

Merged
merged 3 commits into from
Mar 1, 2024
Merged

Conversation

AntoineKM
Copy link
Contributor

What did I change?
I've added a custom error message and a process stop when a 429 error occurs.

Why did I change it?
To partially fix #8, but as we discussed (#8 (comment)), we'll also need to add a progress backup in a future pull request.

@AntoineKM AntoineKM mentioned this pull request Feb 28, 2024
@AntoineKM AntoineKM marked this pull request as draft February 28, 2024 15:56
@goenning
Copy link
Owner

There's already some sort of built-in progress backup using the .cache folder.

If there's a 429 before the .cache is saved, we save the url with status "Error" (or maybe "RateLimited"?), but we don't exit the script.

After the .cache is saved, when we're doing the indexing process, if there's a 429 we notify the user and exit cleanly instead of treating it as an error.

What you think?

@AntoineKM
Copy link
Contributor Author

Yes it can be good I'll try to do it like that

@AntoineKM AntoineKM marked this pull request as ready for review February 29, 2024 15:47
@AntoineKM
Copy link
Contributor Author

@goenning it might be better now what do you think ?

@AntoineKM
Copy link
Contributor Author

Code.mp4

@goenning
Copy link
Owner

goenning commented Mar 1, 2024

Looks great, thank you! :)

@goenning goenning merged commit 9c35961 into goenning:main Mar 1, 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.

429 error
2 participants