Skip to content

Migrate connections protocol to a plugin#925

Closed
dbluhm wants to merge 9 commits intoopenwallet-foundation:mainfrom
dbluhm:feature/connection-protocol
Closed

Migrate connections protocol to a plugin#925
dbluhm wants to merge 9 commits intoopenwallet-foundation:mainfrom
dbluhm:feature/connection-protocol

Conversation

@dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Aug 19, 2024

This PR lifts the connections protocol out of ACA-Py proper and turns it into a plugin. The connections protocol has been deprecated for several releases. This completes the deprecation period while also encouraging the community to move on to bigger and better things (DID Exchange). But, for those who absolutely cannot move on, this plugin will fill the gap.

This is the first instance of a deprecated protocol entering retirement as a plugin. There may be processes to figure out still but I think it's time to make the jump.

Opening as draft while testing for input and thoughts.

cc @swcurran @jamshale

@dbluhm
Copy link
Contributor Author

dbluhm commented Aug 19, 2024

A corresponding PR will be opened to ACA-Py to remove the connections protocol.

@dbluhm
Copy link
Contributor Author

dbluhm commented Aug 19, 2024

Functionality lost by moving connections protocol to a plugin (e.g. things we may wish to reinstate as a feature of this plugin, if there's enough desire for it):

  • Automatic mediator setup on startup using a connections invitation
  • Automatic endorser setup on startup using a connections invitation

Issues now resolved:

  • _fetch_connection_targets_for_invitation needs to be reworked or extended by plugin to support conn invite; consider mapping connections invitation into InvitationMessage for uniform handling.
  • invitation attachment and retrieval; consider storing and retrieving using interface other than conn record
  • request attachment and retrieval; consider storing and retrieving using interface other than conn record

Things currently broken as a result of moving to a plugin (e.g. things we must fix to merge and have "good enough" feature parity):

  • OOB Manager _perform_handshake

@dbluhm
Copy link
Contributor Author

dbluhm commented Aug 19, 2024

Corresponding PR in ACA-Py: openwallet-foundation/acapy#3184

@dbluhm
Copy link
Contributor Author

dbluhm commented Sep 19, 2024

This plugin is dependent on changes in an unreleased version of ACA-Py but is otherwise ready for review.

@dbluhm dbluhm marked this pull request as ready for review September 19, 2024 19:21
Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I'd like to manually test it still and see if there should be anymore integration tests.

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm dbluhm force-pushed the feature/connection-protocol branch from 636de3d to 7fd565e Compare September 27, 2024 19:05
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@swcurran
Copy link
Contributor

Can we merge this?

@jamshale
Copy link
Contributor

@dbluhm Looks like this has become a priority again. Hoping it's not too much work to update. I think the testing is probably the worst because of the new testing profile. Probably easiest to copy them from acapy again. I had updated them all before they got removed recently.

@dbluhm
Copy link
Contributor Author

dbluhm commented Jan 24, 2025

I'm short on time to address this one promptly -- I'd be happy to turn this over to someone else if they're available sooner than I am. Otherwise, I'll get to this as soon as I can, sorry.

@jamshale
Copy link
Contributor

I can probably have a look at it soon. A little bit in limbo with webvh work atm.

@jamshale
Copy link
Contributor

Closing in favor of #1447.

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.

3 participants