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

feat: migrate command for all agents #272

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

iFergal
Copy link
Contributor

@iFergal iFergal commented Jul 18, 2024

This PR adds a keria migrate command that will fetch all of the agents from the agency DB and kick off the migrate doer used in kli migrate. Slightly weird to mimic a kli command from keria but it works, and means we will pick up changes to kli migrate.

I also pinned the keripy version again - this got unpinned by mistake it seems. But as mentioned before we need it for determinism of builds.

Here is a screenshot example of incepting 2-of-2 multi-sig on an old version, trying to connect without migrating and successfully migrating:
image

Note: SignifyGroupHabs are not correctly migrated on keripy - but this is separate for now and has been raised in #257 - I gave some details on why in that issue yesterday.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.10%. Comparing base (18d3ad7) to head (d8e65cc).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
+ Coverage   93.06%   93.10%   +0.04%     
==========================================
  Files          36       36              
  Lines        7121     7221     +100     
==========================================
+ Hits         6627     6723      +96     
- Misses        494      498       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@2byrds
Copy link
Collaborator

2byrds commented Jul 18, 2024

Looks good to me @iFergal. I suppose this is more of an administrative feature, hence the command line tool. Do you have any thoughts how we can test a command like this in case migrate changes?

@2byrds
Copy link
Collaborator

2byrds commented Jul 18, 2024

Also @iFergal could you post a screenshot(s) showing the command being successfully used on this PR?

@iFergal
Copy link
Contributor Author

iFergal commented Jul 18, 2024

Thanks @2byrds I added a screenshot and note to the description.

Yeah based on recommendations from Phil, migrations should be manual, hence the cli.

Regarding testing - I think we would need a minimal agent and set the migration version to the lowest, run keria migrate and then the basic check could be that the version is up to date. I'm not sure if creating an agent would conflict with any schemas that are assumed to be not migrated already (so far, maybe not, but maybe in the future) - but I could look into it.

On the other hand it's probably unlikely that the kli migrate command will be subject to much change since it just opens the DB and calls db.migrate().

@iFergal
Copy link
Contributor Author

iFergal commented Jul 31, 2024

Based on discussions yesterday and WebOfTrust/keripy#824 more thought should be put into this.

If SignifyGroupHab and other files move to KERIA, we may need a separate set of migrations for KERIA on top of calling db.migrate() from keripy.

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