Skip to content
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

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

loneil
Copy link
Contributor

@loneil loneil commented Mar 5, 2024

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.

@swcurran
Copy link
Member

Ping to request this be merged -- @reflectivedevelopment @anwalker293


line = self.readjsonline()

return ""
Copy link
Contributor

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()?

Copy link
Contributor Author

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.

const resp = await agent.oob.deleteById(id);

} catch (error) {
process.stderr.write('ERROR'+ '\n' + error + '\n')
Copy link
Contributor

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:

} catch (e) {

I would suggest, rather than this parse, just throw 'OOB Record not found!'

Copy link
Contributor Author

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

@anwalker293
Copy link
Contributor

^Other than that, LGTM! Kim might have more to say though

Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
@reflectivedevelopment reflectivedevelopment merged commit c476335 into hyperledger:main Mar 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants