openwallet-foundation / acapy

ACA-Py is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://aca-py.org
Apache License 2.0
421 stars 515 forks source link

Update the Alice/Faber OpenAPI (Swagger) demo to show webhooks, remove automation #361

Closed swcurran closed 4 years ago

swcurran commented 4 years ago

A good issue for anyone familiar with the /demo examples. I think the code changes are simple, but happy to discuss.

Update the OpenAPI demo behaviour and script to handle manually all of the connection, issue and prove events via the OpenAPI (Swagger) UI. In conjunction with changing the steps to be done, invoke the demos/run_demo script with two new options:

run_demo --no-auto --events alice

When set the --no-auto parameter would remove all (or perhaps most - as makes sense) of the --auto... options on ACA-Py, so that the controller is forced to respond to and handle each of the protocol events happening in the protocols.

When set the --events parameter would cause the webhook to the controller to be JSON pretty-printed to the console so the user operating the OpenAPI interface would know when an event has occurred and a response is needed.

I think those requirements make sense, but would like to talk with anyone taking this one to make sure we're in sync on what is needed.

I'm happy to do the writing for the update OpenAPI script based on just a bit of guidance on what is needed.

amanji commented 4 years ago

Hi @swcurran I'm happy to take this one on. I'm walking through the code to get an idea of the demo workflow with --auto-accept-invites and --auto-accept-requests off. Is there any other documentation that I should checkout?

swcurran commented 4 years ago

Cool. Unfortunately, there is not a lot. The existing demo is here: https://github.com/hyperledger/aries-cloudagent-python/blob/master/demo/AriesOpenAPIDemo.md

The idea would be to run either or both agents (e.g - https://github.com/hyperledger/aries-cloudagent-python/blob/master/demo/AriesOpenAPIDemo.md#running-the-faber-agent) with the new options and then add in the extra steps in the story that have to be done when the --auto... steps have been removed. I'm hoping that by being able to see the webhook events from the agent, the cutting and pasting can be a bit easier.

Let me know if that makes sense.

amanji commented 4 years ago

@swcurran I have gone through the demo with the --auto-.. flags turned off. There are parts of the issue_credential workflow that are not controlled with those flags.

For example:

When Faber sends a credential offer there is a an auto_issue request body parameter that needs to be set to false, otherwise the credential will automatically be issued to Alice, who then has to explicitly store the credential in her wallet. Prior to this, the handle_issue_credential method in the Alice Agent responds immediately to Faber's credential offer by issuing a credential request back to Faber. Neither of these are controlled by the --auto-.. flags. If we truly want to turn off all automation steps with a --no-auto option I feel there needs to be some control flow implemented in these spots, or the code could be expanded to account for the --auto-.. options. Should I proceed and add the relevant condition checks? Or should we review it further?

swcurran commented 4 years ago

We need to get @andrewwhitehead or @nrempel involved in this. I'll try to touch base with Andrew tomorrow to review this and connect with you.

Were you able to make the --events option work and was it useful? I would think that would be a generally useful, if somewhat noisy capability. How did it work?

This is awesome! Thanks!

amanji commented 4 years ago

I haven't implemented that part yet. Was hoping to get the documentation completed first (which I was able to at least for establishing a connection). Based on the issue description for the --events flag, I'm guessing you're looking for something like an event logger that gets enabled which prints out methods/webhooks that we're called? Please, correct me if I'm wrong here.

swcurran commented 4 years ago

Yes. If I am going to write a controller, and I'm using the Swagger API to learn about the endpoints available for me to call, I'm going to want to see the content of the events I receive. Presumably, a lot of the data I need to use in responding is going to be in the event data I receive.

Does that make sense?

amanji commented 4 years ago

Im just wondering since the Agent modules imports a logger package we could just use that to print out relevant information in the Agent/Alice/Faber runner methods? I was thinking along the lines of a sort of debug info level option that is turned on when the --event flag is set. Perhaps I can fill in those methods with calls to the logger. I'd have to take a look at the documentation for that package.

swcurran commented 4 years ago

@amanji - I that might work. Could it have unintended consequences. E.g. if you associate the webhook data to be displayed when the "debug" level is set, and the "--event" is the equivalent of turning on debug, wouldn't you get lots of other "debug" events? Does logger allow for specified logging type names?

swcurran commented 4 years ago

Also, I checked with Andrew on the --auto... stuff above. He checked and he think it looks right. Perhaps not all of the auto options were removed?

From Andrew:

If auto_issue is not provided as a parameter then the agent defaults to the setting for --auto-respond-credential-request, which should be off

amanji commented 4 years ago

After reading the Logger documentation there are basically three possible approaches:

  1. Create a custom logging level but for events
  2. Creating a custom filter or handler for event logs
  3. A named Logger instance that strictly handles event logs

The first creates additional log levels that deviate from the established standard levels. The second will require filtering through all logs or creating a handler that can capture event logs and could muddy up existing loggers. The third is more modular and encapsulates event log filters/handlers independently of other Logger instances, This named logger can be activated independently of other existing loggers. I am of the belief that the third option is the best approach here.

swcurran commented 4 years ago

Agree that third sounds best.

On Fri, Feb 14, 2020, 7:31 PM Akiff Manji notifications@github.com wrote:

After reading the Logger documentation there are basically three possible approaches:

  1. Create a custom logging level but for events
  2. Creating a custom filter or handler for event logs
  3. A named Logger instance that strictly handles event logs

The first creates additional log levels that deviate from the established standard levels. The second will require filtering through all logs or creating a handler that can capture event logs and could muddy up existing loggers. The third is more modular and encapsulates event log filters/handlers independently of other Logger instances, This named logger can be activated independently of other existing loggers. I am of the belief that the third option is the best approach here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hyperledger/aries-cloudagent-python/issues/361?email_source=notifications&email_token=AAHYRQUSBB3VS3I5GNGWOZDRC5OZZA5CNFSM4KQTKEU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL3APNI#issuecomment-586549173, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHYRQUVWGEG2HEE67MPQ3TRC5OZZANCNFSM4KQTKEUQ .