Feature/event bus

[farskipper]

This adds an event bus and utilizes it to simplify webhooks. Events are emitted and the Admin Server subscribes to some of them to be forwarded as webhooks. Here is a diagram outlining the refactor: https://hackmd.io/IoDqYR_cS3yRAe7DI8JAiA

Events are namespaced. acapy::* events are anything originating from ACA-Py this way plugins can use their own events without collision.

Some notable changes:

Responder.send_webhook

Is deprecated, it emits a warning and uses the acapy::webhook::{TOPIC} event to retain backwards compatibility.

BaseRecord webhooks

Instead of sending a webhook, it now emits an event. As such, WEBHOOK_TOPIC -> RECORD_TOPIC and send_webhook -> emit_event

The record event topic follows this pattern: acapy::record::{RECORD_TOPIC}::{STATE}

Removed Admin Server webhook_targets

It appears that the admin server webhook_targets are no longer used. See: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/admin/server.py#L754-L755 Instead we just rely on profile.settings.get("admin.webhook_urls")

As such, the following are removed:

• BaseAdminServer.add_webhook_target
• BaseAdminServer.remove_webhook_target
• BaseAdminServer.send_webhook

[swcurran]

Something for @shaangill025 @andrewwhitehead and @TimoGlastra to look at it. And anyone else working on ACA-Py, of course!

Thanks!

[airoasis]

Is it resolving the issue #950 as well?

[ianco]

Are there any changes to aca-py startup parameters, or any changes to how the controller interacts with aca-py?

[ianco]

I ran a quick test of the Alice/Faber demo (startup the 2 agents, establish connection, issue credential from faber -> alice and then ask for proof request), it looks like Alice doesn't receive the proof request on the last step.

[dbluhm]

The goal is for the webhook listeners and controller to operate exactly as before. @farskipper we'll have to check out the issues with the demo.

[dbluhm]

Is it resolving the issue #950 as well?

@airoasis This does not impact #950. This change just adds a layer between ACA-Py and the Webhook emitter that enables plugins to process events without needing to run a whole webserver to listen for webhooks.

[farskipper]

@ianco I updated the branch and made a small fix. Now I'm able to get through the Alice/Faber demo.

[andrewwhitehead]

This looks great to me so far, and running the performance demo it seems to be performing 20-30% faster running native on my Mac (it takes longer to finish starting the credential exchanges, but finishes sooner). @ianco Are you able to test the stability on openshift?

[ianco]

This looks great to me so far, and running the performance demo it seems to be performing 20-30% faster running native on my Mac (it takes longer to finish starting the credential exchanges, but finishes sooner). @ianco Are you able to test the stability on openshift?

We need to build a docker image in order to deploy/test on openshift, prob worth doing once we merge this and any other big PR's (for example I'd like to get the shared-components onto openshift as well).

[ianco]

Overall looks really good. Is the EventBus pluggable? Seems like a good fit for the persistent queue.

[dbluhm]

Overall looks really good. Is the EventBus pluggable? Seems like a good fit for the persistent queue.

Interesting question... The current implementation aims to be simple so that components within ACA-Py and attached plugins can receive the same events originally sent only by the admin server via web hooks. Using a plugin, you can subscribe to events and then push those to an external queue or notification system. I'm not sure I see what you mean about using it with the persistent queue yet.

[codecov-commenter]

Codecov Report

Merging #1063 (708f9d3) into main (8a60d1a) will decrease coverage by 0.10%.
The diff coverage is 85.82%.

@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   99.29%   99.19%   -0.11%     
==========================================
  Files         382      383       +1     
  Lines       22099    22133      +34     
==========================================
+ Hits        21943    21954      +11     
- Misses        156      179      +23     

[Luis-GA]

It is a test class for test purposes that only is used on unit tests.
This shouldn't be declared here, isn't it?

[dbluhm]

This follows a pattern used elsewhere in ACA-Py so I don't see a problem with defining it here. This makes it easier to reuse in tests.