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

CLI: Validate storage in verdi storage version #6551

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jul 24, 2024

Fixes #6547

The verdi storage version, in addition to printing the version of the code's and storage's schema, now also validates the storage. If the storage is corrupt or cannot be reached, the command returns the exit code 3. If the storage and code schema versions are incompatible, exit code 4 is returned. This way this command serves as an alternative to running verdi storage migrate as a way to check whether a profile needs to be migrated. The verdi storage migrate command needs to perform checks such as whether the daemon is running and so is always going to be slower.

The `verdi storage version`, in addition to printing the version of the
code's and storage's schema, now also validates the storage. If the
storage is corrupt or cannot be reached, the command returns the exit
code 3. If the storage and code schema versions are incompatible, exit
code 4 is returned. This way this command serves as an alternative to
running `verdi storage migrate` as a way to check whether a profile
needs to be migrated. The `verdi storage migrate` command needs to
perform checks such as whether the daemon is running and so is always
going to be slower.
@sphuber
Copy link
Contributor Author

sphuber commented Jul 24, 2024

@danielhollas would something like this work?

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.

Project coverage is 77.83%. Comparing base (ef60b66) to head (6459592).
Report is 105 commits behind head on main.

Files Patch % Lines
src/aiida/cmdline/commands/cmd_storage.py 88.89% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6551      +/-   ##
==========================================
+ Coverage   77.51%   77.83%   +0.33%     
==========================================
  Files         560      566       +6     
  Lines       41444    41979     +535     
==========================================
+ Hits        32120    32669     +549     
+ Misses       9324     9310      -14     

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

@danielhollas
Copy link
Collaborator

Overall this makes sense, I'll have a closer look later. Also want to do some benchmarks to see if this is indeed an improvement. For a profile without a broker (after #6550), running verdi storage migrate and verdi storage version takes roughly the same amount of time (which makes sense)

`❯ hyperfine -w 5 'verdi storage version' 'verdi storage migrate --force'
Benchmark 1: verdi storage version
  Time (mean ± σ):     737.2 ms ±   4.7 ms    [User: 749.3 ms, System: 812.7 ms]
  Range (min … max):   729.5 ms … 745.8 ms    10 runs
 
Benchmark 2: verdi storage migrate --force
  Time (mean ± σ):     763.7 ms ±   7.3 ms    [User: 779.1 ms, System: 811.5 ms]
  Range (min … max):   758.6 ms … 783.0 ms    10 runs
 
Summary
  verdi storage version ran
    1.04 ± 0.01 times faster than verdi storage migrate --force

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.

Add verdi storage version|status
2 participants