DRAFT: please_ack support PoC for the 0453-issue-credential-v2 protocol #2546
Conversation
Signed-off-by: Alexander Sukhachev <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
Thanks! Awesome. Sorry for the silence on this — we’ve been at a conference this week. We’ll be looking at this next week. Would you be willing to present your design and implementation at the ACA-Pug meeting this coming Tuesday (2023-10-17, 8:00 Pacific / 17:00 Central Europe)? |
Hello @swcurran . Thank you for the answer. I think it's a good idea. I will participate the meeting and share my thoughts/ideas. |
| parser.add_argument( | ||
| "--cred-issue-ack-required", | ||
| action="store_true", | ||
| env_var="ACAPY_CRED_ISSUE_ACK_REQUIRED", |
There was a problem hiding this comment.
This raises another question -- how to define configuration settings for when to request ACKs? For example, is there a way with ArgParser to add a parameter type --ack-request-protocol <protocol> that can be defined multiple times? I don't know if that is even possible...
Likewise, should the Admin API be extended for supported protocols to add a add-please-ack on certain endpoints? That would mean that the controller would be responsible for using please ack when necessary/desirable.
| @@ -667,6 +679,36 @@ async def send_cred_ack( | |||
|
|
|||
| return cred_ex_record, cred_ack_message | |||
|
|
|||
|
|
|||
| async def transit_to_done( | |||
There was a problem hiding this comment.
Is this "DONE" in the context of the entire protocol, or just in the context of the current message for which a "please-ack" OUTCOME was requested?
Or to put another way -- does each please-ack-aware protocol need to declare -- these are the things for which I am willing to ACK. For example, for issue one, I think:
- If auto is set on, then no "RECEIPT" acks are except on "Issue" from the Issuer to the Holder (Holder responds)
- If auto is not set on, the RECEIPT" acks whenever requested
- OUTCOME Ack only from Holder to Issuer on completion of processing for the "Issue" message.
In other words -- only send Acks where there is not already a message going back to the other party at the same time.
What do you think?
There was a problem hiding this comment.
Hello @swcurran. Thank you for your questions.
-
This "DONE" is in the context of the protocol. Holder decides if it's time to send an 'ack' message or just to change state without sending it (it depends on whether an issuer has sent a 'please_ack' or not).
-
I was thinking about your idea (decide to send an explicit 'ack' or not depending on whether there is expected messages in response to message containing a 'please_ack' or not). I see it is not mentioned in the please_ack RFC. There is only one sentence that implies that it's ok to ignore 'please_ack' in some cases:
agents also need the ability to request additional ACKs at other points in an instance of a protocol. Such requests may or may not be answered by the other party, hence the "please" in the name of decorator.
It sounds we can ignore the 'please_ack' when an agent is going to send response immediately. But first of all, it is only possible for 'RECEIPT', not for 'OUTCOME' (we discussed it here ). At the other hand, I think this behavior (sending explicit 'ack' message or not depending on some conditions) has to be defined in the 'please_ack' RFC.
My proposal is to continue implementation of support for 'OUTCOME'. And maybe to start discussion (in background with community) about possible behavior for agents who received 'RECEIPT' (is it ok to ignore in some cases or not? If yes, in what cases exactly? etc). Does it sound reasonable in your opinion?
There was a problem hiding this comment.
See my comment on the Markdown file PR that I just posted. I’ve changed my mind about sending ACKs only sometimes — I think that does not make sense and is too hard to implement. My new thinking is that the use of ~please_ack should have no impact on the states, messages and processing of a protocol — it should just result in an extra message or two being sent at the appropriate time.
|
Note the lint and failed PR test(s). |
|
The please_ack decorator has been removed from AIP 2. We are unlikely to implement this within ACA-Py in the near future, if at all, without revisiting the decorator and it's behavior in the RFCs. |
This PR is PoC code for the PR#2540 (#2540). It shows possible implementation of the
please_ackdecorator support ('option 2' in the document in the PR#2540)