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

Remove retry and fix tests #2392

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Conversation

roman-opentensor
Copy link
Contributor

@roman-opentensor roman-opentensor commented Nov 5, 2024

The historical idea of ​​retry was that regardless of any Errors, it repeated its attempts 3 times.
Now the SDK code is much more functional and cleaner. I catch different errors at different places.

  1. ConnectionRefusedError error is caught during the initialization of the Subtensor
  2. The connection failure error is handled by decorator @networking.ensure_connected
  3. Only the subtensor response error remains based on the data passed by the user or based on the current state of the subtensor. In this case, if the first response was an error, then all three attempts return exactly the same result. We are just unnecessarily loading the chain with additional calls. We tested this behavior many times. This is the reason why we abandoned retry in the async impl of the Subtensor and AsyncSubstrateInterface.

Also, an important point is that SDK is a Python package that is intended for use in scripts. It is not an interactive package. All responsibility for processing the results is in the hands of the user. This should be considered as a more flexible way then in btcli. The user will be able to regulate the retries themselves if they want. If the data was wrong during the request and the response is not successful, then it is easier to process and make a request based on other data than to make 3 calls and then change the data.

@roman-opentensor roman-opentensor self-assigned this Nov 5, 2024
@roman-opentensor roman-opentensor requested a review from a team November 5, 2024 21:48
Copy link
Contributor

@thewhaleking thewhaleking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@roman-opentensor roman-opentensor merged commit 22e1557 into staging Nov 5, 2024
24 checks passed
@roman-opentensor roman-opentensor deleted the feat/roman/remove-retry branch November 5, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants