Skip to content

Improvements in rpc.py #9427

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Improvements in rpc.py #9427

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba30669
Improvements Made:
Adriel007 Aug 7, 2024
5ec8cbf
timeout increased
Adriel007 Aug 8, 2024
80084e7
try catch removed
Adriel007 Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 33 additions & 38 deletions utils/python-rpc/framework/rpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Copyright (c) 2018-2024, The Monero Project

#
# All rights reserved.
#
Expand Down Expand Up @@ -33,56 +32,52 @@

class Response(dict):
def __init__(self, d):
for k in d.keys():
if type(d[k]) == dict:
self[k] = Response(d[k])
elif type(d[k]) == list:
self[k] = []
for i in range(len(d[k])):
if type(d[k][i]) == dict:
self[k].append(Response(d[k][i]))
else:
self[k].append(d[k][i])
for k, v in d.items():
if isinstance(v, dict):
self[k] = Response(v)
elif isinstance(v, list):
self[k] = [Response(i) if isinstance(i, dict) else i for i in v]
else:
self[k] = d[k]
self[k] = v

def __getattr__(self, key):
return self[key]
try:
return self[key]
except KeyError:
raise AttributeError(f"'Response' object has no attribute '{key}'")

def __setattr__(self, key, value):
self[key] = value
def __eq__(self, other):
if type(other) == dict:
return self == Response(other)
if self.keys() != other.keys():
return False
for k in self.keys():
if self[k] != other[k]:
return False
return True

class JSONRPC(object):
def __init__(self, url, username=None, password=None):
class JSONRPC:
def __init__(self, url, username=None, password=None, timeout=20):
self.url = url
self.username = username
self.password = password
self.timeout = timeout

def send_request(self, path, inputs, result_field=None):
try:
response = requests.post(
f"{self.url}{path}",
data=json.dumps(inputs),
headers={'content-type': 'application/json'},
auth=HTTPDigestAuth(self.username, self.password) if self.username else None,
timeout=self.timeout
)
response.raise_for_status()
except requests.exceptions.RequestException as e:
raise SystemExit(f"Request failed: {e}")
Copy link

Choose a reason for hiding this comment

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

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).

Copy link

Choose a reason for hiding this comment

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

To clarify: I think raising ValueError here rather than asserting is a good change, but would suggest not catching the RequestException.


res = response.json()

def send_request(self, path, inputs, result_field = None):
res = requests.post(
self.url + path,
data=json.dumps(inputs),
headers={'content-type': 'application/json'},
auth=HTTPDigestAuth(self.username, self.password) if self.username is not None else None)
res = res.json()

assert 'error' not in res, res
if 'error' in res:
raise ValueError(f"Error in response: {res['error']}")

if result_field:
res = res[result_field]
return Response(res.get(result_field, {}))
Copy link

Choose a reason for hiding this comment

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

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.


return Response(res)

def send_json_rpc_request(self, inputs):
return self.send_request("/json_rpc", inputs, 'result')



return self.send_request("/json_rpc", inputs, 'result')
Loading