Cannot migrate due to container folder being present

[CasperWA]

Describe the bug

The issue of not being able to migrate an AiiDA database that should've been fixed in #4889 might've been undone by #4891.

I keep getting the following error (even after deleting the container folder):

$ rm -rf /home/candersen/.aiida/repository/li-ion-conductors/container
$ verdi -p li-ion-conductors database migrate -f
...
Operations to perform:
  Apply all migrations: auth, contenttypes, db
Running migrations:
Critical: the container /home/candersen/.aiida/repository/li-ion-conductors/container already exists. If you ran this migration before and it failed simply delete this directory and restart the migration.
  Applying db.0047_migrate_repository...
$

Steps to reproduce

Steps to reproduce the behavior:

Migrate a database using the latest develop commit (fa8d05f)

Expected behavior

Successful migration.

Your environment

• Operating system: Ubuntu 20.04
• Python version: 3.8.5
• aiida-core version: fa8d05f

[CasperWA]

I think the issue is the removal of the else-clause here, although I was originally of the impression that #4891 would fix and remove the need for having this else-clause.

[sphuber]

Hmm, that is pretty bad if this fails again, and especially that we are not catching this in our tests. Just to make sure, if you delete the folder /home/candersen/.aiida/repository/li-ion-conductors/container and run the database migration command again, you get the same error every time?

I think the issue is the removal of the else-clause here, although I was originally of the impression that #4891 would fix and remove the need for having this else-clause.

I removed this intentionally, because now the uuid property should return None if the repo is not initialised, and this is properly tested on the Repository class level.

[mbercx]

I think perhaps the Profile.get_repository() method should not initialise the repository?

[CasperWA]

Hmm, that is pretty bad if this fails again, and especially that we are not catching this in our tests. Just to make sure, if you delete the folder /home/candersen/.aiida/repository/li-ion-conductors/container and run the database migration command again, you get the same error every time?

Yes. I also updated the OP to reflect this.

I think the issue is the removal of the else-clause here, although I was originally of the impression that #4891 would fix and remove the need for having this else-clause.

I removed this intentionally, because now the uuid property should return None if the repo is not initialised, and this is properly tested on the Repository class level.

That's what I thought.

[mbercx]

I think perhaps the Profile.get_repository() method should not initialise the repository?

Is there a reason why the repository should be initialised by default? Can this not simply be done when migrating/setting up a profile?

[sphuber]

I think perhaps the Profile.get_repository() method should not initialise the repository?

Is there a reason why the repository should be initialised by default? Can this not simply be done when migrating/setting up a profile?

Yes, I think this is at the root of the problem. Will make a PR, but I think we really need to come up with a test to catch this, but I don't have ideas how yet.

[mbercx]

I suppose we'd add tests at the Profile level? I.e. add tests to this class:

aiida-core/tests/manage/configuration/test_profile.py

Lines 21 to 22 in fa8d05f

	class TestProfile(AiidaTestCase):
	"""Tests for the Profile class."""

After the profile has been set up with the constructor here, the repository should not have been initialised. So then you can check that the uuid is None?

[sphuber]

That would only test that Profile.get_repository no longer initialises the repository though. The problem is that other code can still initialise it which would then still cause migrations to fail. We really need to test the migration pathway here as a normal user would migrate an existing database.

[giovannipizzi]

Can we have a test similar to import/export, but instead of importing from a static .aiida file, we put in the DB (and file repo) the content of (small) DB and repo dumps, created with AiiDA 1.6.x (and stored in the same way of the .aiida files). Then, we try to migrate and check that afterwards everything works (container is there, we can still get the content of a file of a node)