-
Notifications
You must be signed in to change notification settings - Fork 13
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
Delete OOB utility function #47
Conversation
Ping to request this be merged -- @reflectivedevelopment @anwalker293 |
load-agent/locustClient.py
Outdated
|
||
line = self.readjsonline() | ||
|
||
return "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to return anything
Also, just for a sanity check, has this been tested / shown to work? - Do we need the
line = self.readjsonline()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that line
in line = self.readjsonline()
is unused, but I'd seen that in a few other methods here as well (where line
isn't used either, ping_mediator
, receive_credential
, presentation_exchange
for example) so left it in.
Then if I take it out I started getting errors like
load-agent-1 | [2024-03-21 16:56:13,560] load-agent/ERROR/locust.user.task: 'NoneType' object is not subscriptable
load-agent-1 | Traceback (most recent call last):
load-agent-1 | File "/usr/local/lib/python3.8/dist-packages/locust/user/task.py", line 347, in run
load-agent-1 | self.execute_next_task()
load-agent-1 | File "/usr/local/lib/python3.8/dist-packages/locust/user/task.py", line 372, in execute_next_task
load-agent-1 | self.execute_task(self._task_queue.pop(0))
load-agent-1 | File "/usr/local/lib/python3.8/dist-packages/locust/user/task.py", line 384, in execute_task
load-agent-1 | task(self)
load-agent-1 | File "/load-agent/load-agent/locustMediatorIasIssueDeleteOob.py", line 62, in delete_oob_record
load-agent-1 | self.client.delete_oob(self.connection["oobRecordId"])
load-agent-1 | TypeError: 'NoneType' object is not subscriptable
hadn't dug into the cause so left it in like the other methods.
The return
is indeed unneeded so I'll take that out.
load-agent/agent.ts
Outdated
const resp = await agent.oob.deleteById(id); | ||
|
||
} catch (error) { | ||
process.stderr.write('ERROR'+ '\n' + error + '\n') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format throughout agent.ts
has been to throw the error directly. If you throw the error, it gets caught by this line:
aries-akrida/load-agent/agent.ts
Line 566 in a327e04
} catch (e) { |
I would suggest, rather than this parse, just throw 'OOB Record not found!'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking this try/catch out to let it propagate up. 👍
^Other than that, LGTM! Kim might have more to say though |
de42e9e
to
36c4da8
Compare
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
36c4da8
to
ebbe208
Compare
For one of the BC Gov IAS testing cases we delete the OOB record so an agent can accept the same invitation again. In that I'd added a call for the AFJ agent to do this.
It's very bare bones compared to the other connection management calls, doesn't do a defer to wait for state changes, but that may(?) be appropriate for a simple delete record call the way this is invoking AFJ
oob.deleteById
Would be good to pull this simple call in at least so the BCGov IAS tests don't need manual/forked Akrida changes to run, if more feature/additional complexity needed could iterate on it later.