Skip to content

Conversation

@dharmeshkakadia
Copy link
Contributor

Notion frequently throws 500-504 errors, especially for large collections and pages. While there is existing suggestion (#234), I agree with the comment in the code, its risky to retry non-idempotent behaviour.

This PR will allow clients to specify the retry object as part of the client initialisation, since client has the best knowledge about retry safety. It's a new optional parameter and if not specified, it will use the current retry logic. I have migrated a a collection with 40000+ rows and 50000 pages linking into it with this changes. Thoughts?

@dharmeshkakadia
Copy link
Contributor Author

I can add relevant doc and example if needed.

@jamalex
Copy link
Owner

jamalex commented Jan 25, 2021

Thanks, @dharmeshkakadia! While I do wonder about whether it might be good to make something more robust/aggressive as the default, this seems like a good middleground for now.

A followup PR with a short example (and note of when it might be good to use) for the README would be great! Merging, in the meantime.

@jamalex jamalex merged commit be836af into jamalex:master Jan 25, 2021
@dharmeshkakadia dharmeshkakadia deleted the client_specified_retry branch April 6, 2021 21:47
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.

2 participants