Add the -v/--version option to verdi export migrate

[sphuber]

Fixes #3909

The default behavior remains the same and if not specified the export
archive will be migrated to the latest version. However, with the flag
any other version can be chosen, as long as it constitutes a forward
migration as backward migrations are not supported.

[CasperWA]

Thanks @sphuber.

Good to see that all cmdline stuff is pulled out of the importexport module!

This seems to be a nice little "fix" for stopping underway in the migration as well.

There are some other migration tests that seem to fail, but other than that I only found a small "typo" and otherwise approve this change.

[CasperWA]

Suggested change

	:param version: the version to migrate to, by default is the current export version
	:param version: the version to migrate to, by default the current export version

or it is.

[CasperWA]

Since you're adding functionality to migrate_recursively could you also make a test for the new feature, directly testing the function?

Furthermore, the failing test (that I see you have now fixed) was originally a request by @ltalirz that upon failure the exit code should be non-zero. Can you confirm that this is still the case? (Since we're raising in all cases, I guess this is still the case, but just to be sure.) There should probably already be another test for this?

[sphuber]

Since you're adding functionality to migrate_recursively could you also make a test for the new feature, directly testing the function?

Good point, will add it

Furthermore, the failing test (that I see you have now fixed) was originally a request by @ltalirz that upon failure the exit code should be non-zero. Can you confirm that this is still the case? (Since we're raising in all cases, I guess this is still the case, but just to be sure.) There should probably already be another test for this?

The behavior is still the same, I simply made the utility function raise instead of echo'ing directly. The CLI now catches this exception and turns it into a critical echo. We should not include echo functionality in non-CLI code.

[codecov]

Codecov Report

Merging #3910 into develop will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #3910   +/-   ##
========================================
  Coverage    78.00%   78.01%           
========================================
  Files          457      457           
  Lines        33708    33714    +6     
========================================
+ Hits         26294    26301    +7     
+ Misses        7414     7413    -1     

Flag	Coverage Δ
#django	70.01% <100.00%> (+<0.01%)	⬆️
#sqlalchemy	70.85% <100.00%> (+<0.01%)	⬆️

Impacted Files	Coverage Δ
aiida/cmdline/commands/cmd_export.py	92.50% <100.00%> (+0.39%)	⬆️
aiida/transports/plugins/local.py	80.46% <0.00%> (+0.25%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 676a83e...08f38d0. Read the comment docs.

[CasperWA]

Furthermore, the failing test (that I see you have now fixed) was originally a request by @ltalirz that upon failure the exit code should be non-zero. Can you confirm that this is still the case? (Since we're raising in all cases, I guess this is still the case, but just to be sure.) There should probably already be another test for this?

The behavior is still the same, I simply made the utility function raise instead of echo'ing directly. The CLI now catches this exception and turns it into a critical echo. We should not include echo functionality in non-CLI code.

I completely agree, as per my original review comment.
I just wanted to mention it (and tag @ltalirz) and make doubly sure it still satisfies his specific use case :)

[CasperWA]

Thanks for the additional test.

Since I anyway found a wrong comment in the test you updated (in fact, you may remove the comment completely), I also added another suggested change - just because :)

Just ping me after the change and I'll approve.

[ltalirz]

I'm sure raising an exception/echo_critical will result in a nonzero exit code, so no complaints from my side.

[sphuber]

@CasperWA done and done

[CasperWA]

Thanks @sphuber ! Merge away 🚀