Open aliqandil opened 1 year ago
Hi @aliqandil , thank you for using this library as well as the issue submission!
I see your usecase and agree it would be a fine addition.
Besides a little bit of a clean-up, a thing to consider would be also the behaviour of this coroutine method when things go wrong.
Your code snippet implements the return_exceptions=True
approach, meaning coroutine never raises, it treats RPCError
exceptions as ordinary values and returns a list of either RPCError
or the dictionary representing a valid result, ie List[Union[RPCError, dict]]
.
Another approach would be to raise on a first error encountered, meaning if the coroutine successfully returned, you are certain the list will not contain RPCError
objects, so the signature would be List[dict]
.
In code, we could represent it like this:
async def batch(
self,
methods_with_params: List[Tuple[str, List[JSONType]]],
return_exceptions: bool = True,
**kwargs: Any,
) -> List[Union[RPCError, BitcoinRPCResponse]]:
request_list = [
{
"jsonrpc": "2.0",
"id": self._counter(),
"method": method,
"params": params,
}
for method, params in methods_with_params
]
response = await self.client.post(
url=self.url,
content=orjson.dumps(request_list),
**kwargs,
)
# Raise an exception if return code is not in 2xx range
# https://www.python-httpx.org/quickstart/#exceptions
response.raise_for_status()
content = orjson.loads(response.content)
if return_exceptions:
return [
RPCError(d["error"]["code"], d["error"]["message"])
if d["error"]
else d["result"]
for d in content
]
else:
# TODO: Maybe introduce an ExceptionGroup, since multiple calls may fail?
# TODO: Would it be worth returning the results that did not fail as well?
result = []
for d in content:
if d["error"]:
raise RPCError(d["error"]["code"], d["error"]["message"])
else:
result.append(d)
return result
Does it make sense? Is it okay for you to have it included in the library (with minor version bump) this or the following weekend?
As I said, the behaviour may be tricky when errors are present and I would like to have it covered by some testcases.
Best regards, Libor
The title is pretty self explanatory, I'm not familiar enough with the project to create a pull request, but this is the patch I use to get this functionality: