-
Notifications
You must be signed in to change notification settings - Fork 435
Feat/centralized retry #3227
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
base: staging
Are you sure you want to change the base?
Feat/centralized retry #3227
Conversation
|
@basfroman, please, I would love to get your review on this |
|
All CI fixes have been applied:
@basfroman, please can you re-review |
|
The code seems decent, but I'm not sure this is a good thing. Generally speaking, an SDK should not be opinionated. One of the big things we got away from in SDKv10 was error handling within the SDK. We decided it's best to leave it up to users on how to handle exceptions/errors/retrying. This very much seems like a step back into that opinionated versions of old. |
1adebf1 to
5e3c65f
Compare
This PR has been updated to remove all default retry behavior from core SDK components, while **preserving the retry utility as an optional helper. What Changed1. Retry Utility Remains Available
The retry module is now a user-level helper, intended for developers who want a shared, consistent retry mechanism without re-implementing it themselves. 2. Retry Removed from Core SDK PathsThe SDK itself no longer applies retries anywhere:
This ensures:
3. Defensive Bug Fix RetainedThe fix to
This change is a bug fix, not retry logic, and is independent of the retry discussion. Resulting Behavior
|
|
Hi @Dairus01, I'll review this PR when I get the time. But what @thewhaleking said in this comment is 100% in line with our plan. |
Thanks, I would be looking forward to your review because, after @thewhaleking comment, I now made the retry optional. Instead of a centralised retry, it is now an optional retry that you can implement if you want to enable it. You can simply enable it, allowing for a maximum retry attempt, a base delay, a maximum base delay, and other features. I would be happy to improve anything required. |
Sounds good for me. I'll back to this PR on the next Monday. |
I would be looking forward to it |
This PR introduces a centralized, opt-in retry and backoff
mechanism for network operations in the Bittensor SDK.
Key points:
(Dendrite and Subtensor)
disabled behavior, and default exception safety
This change improves SDK reliability under transient network
failures without affecting protocol logic or consensus behavior.
Contribution by Gittensor, learn more at https://gittensor.io/