Improvements in rpc.py #9427
Improvements in rpc.py #9427
Conversation
- **Using `isinstance`**: - The use of `isinstance` improves readability and is the recommended way to check types. - **Exception Handling**: - Added checks and exception handling to deal with network failures and other possible exceptions during the request. - **Timeout**: - Added a timeout option to prevent the code from hanging indefinitely waiting for a response. - **Code Readability Improvement**: - General improvements in code readability, making it clearer and easier to understand.
utils/python-rpc/framework/rpc.py
Outdated
| ) | ||
| response.raise_for_status() | ||
| except requests.exceptions.RequestException as e: | ||
| raise SystemExit(f"Request failed: {e}") |
There was a problem hiding this comment.
Raising SystemExit in an RPC client tends to be counterproductive: Better to inform the caller of the actual error (which, in this case, would mean keeping the original exception, unless you can extract more precise information, such as the old client's check for an error key).
There was a problem hiding this comment.
To clarify: I think raising ValueError here rather than asserting is a good change, but would suggest not catching the RequestException.
|
|
||
| if result_field: | ||
| res = res[result_field] | ||
| return Response(res.get(result_field, {})) |
There was a problem hiding this comment.
This looks like a subtle, and incorrect, behavior change: The previous code would raise a KeyError if the response doesn't contain result_field, whereas the new code returns an empty response, thereby silently ignoring malformed responses. I would suggest reverting this change.
Using
isinstance:isinstanceimproves readability and is the recommended way to check types.Exception Handling:
Timeout:
Code Readability Improvement: