Automate export file migration for verdi import
#2820
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CasperWA
force-pushed
the
fix_2739_auto_migrate_verdi_import
branch
from
773ebf5
to
c4ea7fb
Compare
CasperWA
force-pushed
the
fix_2739_auto_migrate_verdi_import
branch
2 times, most recently
from
c1e19c5
to
930e767
Compare
ltalirz
reviewed
May 10, 2019
ltalirz
reviewed
May 10, 2019
ltalirz
reviewed
May 10, 2019
ltalirz
reviewed
May 10, 2019
ltalirz
reviewed
May 10, 2019
ltalirz
suggested changes
May 10, 2019
There was a problem hiding this comment.
thanks @CasperWA , code duplication already went down a lot!
I think there is a bit more to reduce in cmd_import but this does not need to be done in this PR
For reference, I would suggest:
- archives should simply be processed one by one (there is no need to create and validate the full list of archives before you even start. e.g.
rm file1 file2 file3also will start removing files and only stop when it encounters a file that isn't there) - when you look at an archive you figure out whether it's a url or not. if url, validate URL and call function to download. you replace url by path to downloaded file and the following code runs inside a sandbox folder
- from now on all code is shared...
Fixes aiidateam#2739. Invoke `verdi export migrate` in `verdi import` if it turns out the export file archive cannot be imported due to an IncompatibleArchiveVersionError. The default is to automatically migrate it (via a temporary file). Introduce the `--non-interactive` flag for `verdi import`. Add tests for new functionality. Make sure `--non-interactive` does not print confirm messages. Make sure old export files are automatically migrated, both local files and from URLs. Introduce the `--migration`/`--no-migration` choice to force migration, if needed or avoid automatic migration. This makes sure that an error code != 0 is still present, when automatic migration is not performed on old export files. Following behaviour is expected and implemented: | `migration` | `non_interactive` | Result CLI | Result script | | ------------- | ---------------------- | ------------- | ---------------- | | `True`* | `False`* | Query user, migrate | Migrate | | `True`* | `True` | No query, migrate | Migrate | | `False` | `False`* | No query, no migrate | No migrate | | `False` | `True` | No query, no migrate | No migrate | *Default. Separate code into two utility functions that help to import and migrate.
CasperWA
force-pushed
the
fix_2739_auto_migrate_verdi_import
branch
from
930e767
to
caa7efc
Compare
ltalirz
approved these changes
May 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2739.
Invoke
verdi export migrateinverdi import, if it turns out the export file archive cannot be imported due to anIncompatibleArchiveVersionError.The default is to automatically migrate it (via a temporary file).
Introduce the
--non-interactiveflag forverdi import.This will avoid printing confirm messages.
Add hidden
--manual-migrationflag to turn off automatic migration.This is hidden, because it should only be used for tests, in order to switch the default to not do automatic migration. This should also make sure that an error code != 0 is still present when automatic migration is not performed on old export files.