Conversation
basfroman
left a comment
There was a problem hiding this comment.
LGTM with a few questions
| self.rpc_request( | ||
| method="state_queryStorageAt", | ||
| params=[batch_keys, block_hash], | ||
| runtime=runtime, | ||
| ) |
There was a problem hiding this comment.
What happens if one of the self.rpc_requests raises an error? For example, due to a short, temporary bad connection? Or a broken data structure?
You don't want to use return_exceptions=True for the gather and then filter the result? or use try except with logging in case of error?
There was a problem hiding this comment.
No, I'd rather not. The problem with returning exceptions is that we would have one of three options at that point:
- log the exception, and return partial data
- Return no data, and re-raise the exception
- Retry the failed calls
By not returning the exception, we're implicitly choosing option 2. Option 1 would be fine if we trusted people to correctly examine logs before moving on. However, in the case where most of this will not be watched before being used, we cannot rely on someone to say "okay, this is incomplete data, what now must I do with it?", and also the fact that this only occurs when using fully_exhaust=True, which is stated to only be used if you want all (read: not partial) data. Therefore, I think this is the best option.
Option 3 doesn't make sense, because those calls are already retried if the exception is due to a websocket timeout, and if not can be caught be something else (such as with RetryAsyncSubstrate.
There was a problem hiding this comment.
In other words, we only consider the case where ALL results are successful or we get an error. Right?
Adds an arg to async substrate query map to fully exhaust the query map results rather than synchronously getting them by pagination.
In bigger tests such as
SubtensorModule.OwnedHotkeys, the speed improvement is 5-6X faster.