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

PR#1543 Follow up - Adding invitation_msg_id and their_public_did back to record_value. #1553

Merged
merged 2 commits into from
Dec 13, 2021

Conversation

shaangill025
Copy link
Contributor

Signed-off-by: Shaanjot Gill [email protected]

@shaangill025
Copy link
Contributor Author

@swcurran Based on discussion with Andrew and Ian regarding this link, invitation_msg_id and their_public_did needs to be added back to record_value for 7.3rc0 release. For backward compatibility, the previously created ConnRecord would need to be re-saved so that it can be reused. Andrew sees no real benefit to doing this as old connections cannot be reused but new ones can be established.
Regarding updating existing data or handling breaking changes, based upon Ian's suggestion, I am working on an ACA-Py upgrade command. I will provide more details on it separately.

@swcurran
Copy link
Contributor

Sounds good, @shaangill025 . Let me know what is "must have" for 0.7.3-rc0 is ready and merged, and what can wait. In the Changelog, I added the following about the previous change. Is that good enough to go forward for 0.7.3 (final), or do you think we should include the "upgrade" process in 0.7.3?

Thanks!

For those that have an existing deployment of ACA-Py with long-lasting connection records, some action
may be required in order to use RFC 434 Out of Band and the "reuse connection" as the invitee. In PR #1453
(details below) a performance improvement was made when finding a connection for reuse. The new approach
(adding a tag to the connection to enable searching) applies only to connections made using this ACA-Py
release and later, and "as-is" connections made using earlier releases of ACA-Py will not be found as reuse
candidates. We don't think this will affect (m)any deployments and so have not implemented a migration
approach. However, if this issue might affect your deployment, please submit an issue and the ACA-Py
team will work with you to find a solution.

Signed-off-by: Shaanjot Gill <[email protected]>
@shaangill025
Copy link
Contributor Author

shaangill025 commented Dec 11, 2021

@swcurran This is ready and the changelog looks good for 7.3.0 final. We don't need the upgrade command for this change to ConnRecord. Ian just mentioned that new tags are being added to credential_exchange (most probably in PR#1545) for which backward compatibility needs to be managed but I don't think it is planned for this release. So, the upgrade process can wait till the subsequent release.

@swcurran
Copy link
Contributor

Sounds good -- thanks @shaangill025 . @ianco and @andrewwhitehead -- please review and merge this (if OK) when you get a chance.

@andrewwhitehead
Copy link
Contributor

andrewwhitehead commented Dec 13, 2021

Is this the only change? Does the connection lookup code need updating, or was it trying to filter on tags that weren't defined? [edit] Nevermind, I see this is just a follow-up.

@swcurran swcurran merged commit 3d286f0 into openwallet-foundation:main Dec 13, 2021
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