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

chore: propose official deprecations of a couple of features #2856

Merged

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Mar 22, 2024

With this PR I'd like to propose that we move the following features to deprecated status:

  • Aries RFC 160: Connection Protocol
  • Aries RFC 0036: Issue Credential 1.0
  • Aries RFC 0037: Present Proof 1.0
  • did:sov:... as Protocol Doc URI

By moving these to deprecated status, we inform the community that these are features that should no longer be used and steer them to DID Exchange and using didcomm.org as the doc uri in their projects.

These items should more or less already be considered deprecated but there are some final efforts that need to be made to tie up loose ends. With the introduction of qualified DIDs in the pending 0.12.0 release and automatically always emitting https://didcomm.org (by setting the flag for it to be permanently set to true), we move close enough to these loose ends being tied off to, in my opinion, move these features to official deprecated status.

In practice, this PR affects the code in the following ways:

  • Prints a deprecation notice on startup (to stderr) that these features are now deprecated.
  • When a message with type starting with did:sov: is received, log a deprecation warning
  • When a connection protocol operation is taken, log a warning message (this one might be a bit aggressive; I'd be happy to tone this back if it's too much)
  • Mark the Admin API endpoints as deprecated in the OpenAPI/Swagger spec and UI. The endpoints are still usable but are greyed out.
    • To clarify, only the Admin API endpoints triggering connection protocol specific operations are marked as deprecated. Other connection management endpoints are NOT marked as deprecated (e.g. listing connections, deleting a connection, etc.).

Let me know your thoughts.

@dbluhm dbluhm requested a review from swcurran March 22, 2024 21:55
@dbluhm dbluhm changed the title chore: propose official deprecations chore: propose official deprecations of a couple of features Mar 22, 2024
@swcurran
Copy link
Contributor

👍 from me — nice work. Long overdue.

I wonder about putting in the did:sov… one since there is no way to use it. The setting is now forced to be the new one. I don’t think a controller can do it via the admin API, so I don’t think this is necessary. Not harmful, but…

I do think we should soon go down the path of turning deprecated protocols into plugins — 0160 Connections and v1 of the Credential Exchange protocols.

Thanks!

@swcurran
Copy link
Contributor

Approving, but won’t merge immediately in case you want to remove the did:sov… warning. But fine if you think it best to leave it.

@dbluhm
Copy link
Contributor Author

dbluhm commented Mar 22, 2024

Yeah, the did:sov one only applies to receiving messages with that type, since we're always sending that type. I think it's good to bring it to the attention of ACA-Py operators if one of their clients are unwittingly using outdated software. Gives them an opportunity to investigate before things eventually break when the support is removed.

On Issue Credential v1, does that meet the threshold for deprecation? I can throw it on the list 😄

@swcurran
Copy link
Contributor

I’d say clarify the message a bit to say that it was received. What about something like:

“Received a core DIDComm protocol with the deprecated `did:sov:BzCbsNYhMrjHiqZDTUASHg;spec` prefix."
                “The sending party should be notified that support for receiving such messages will be removed"
                “in an upcoming ACA-Py release. Use https://didcomm.org/ instead."

My $0.02CDN is that we might as well add v1 Credential Exchange protocols. Start the process...

@dbluhm
Copy link
Contributor Author

dbluhm commented Mar 22, 2024

Sounds good to me; I'll make those adjustments

@dbluhm dbluhm force-pushed the chore/deprecation-warnings branch from cc72110 to 2fccfb1 Compare March 25, 2024 18:54
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@dbluhm
Copy link
Contributor Author

dbluhm commented Mar 25, 2024

Addressed the feedback on the prefix warning and added notices for ICv1 and PPv1. Let me know if there's any other changes you'd recommend 🙂

@swcurran swcurran merged commit f1bb749 into openwallet-foundation:main Mar 25, 2024
8 checks passed
@dbluhm dbluhm deleted the chore/deprecation-warnings branch March 25, 2024 20:43
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.

2 participants