aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
431 stars 187 forks source link

`SqliteZipBackend.create_profile` should automatically define the `default_user_email` #6165

Closed sphuber closed 6 months ago

sphuber commented 11 months ago

Even though this is a read-only backend, it should still set the default_user_profile on the created profile, because otherwise creating any Node will except, even if not storing. See https://aiida.discourse.group/t/setting-up-a-user-for-sqlitezipbackend/139 for example report.

sphuber commented 6 months ago

It will not just except on node creation, but anytime the default user is called. This can also happen for example when calling verdi group list

GeigerJ2 commented 6 months ago

The issue when calling verdi group list with a profile created with the SqliteZipBackend was one of the issues mentioned to me by @giovannipizzi, as well. I suppose you meant default_user_email rather than default_user_profile in your original comment?

Alternatively, rather than setting the existing user profile as the default user, one could also set the default_user_email properly, as the email is asked anyway in the default creation, no? I assume people might still want to create separate, temporary profiles when exploring read-only archives if they already have a main profile? Suddenly having some user from the archive as the default might be confusing.

giovannipizzi commented 6 months ago

Indeed, setting one of the users from the archive is an option, but a bit counterintuitive. Is the problem with the default user only eg in the cmdline? We could change the way we get the email, so it does not except (now the method to get the default user returns None instead of a User class when the email does not correspond to any user in the DB, thus calling the property to get the email excepts). Maybe we should have a different method to get the email that returns None and does not except, and then we can check all the code where we use the user and see how often we do it, and if we can rethink the way we use the default user. So that it becomes OK to have a user email set in the profile config.json that however does not exist in the database.

sphuber commented 6 months ago

I suppose you meant default_user_email rather than default_user_profile in your original comment?

Yes, I meant default_user_email.

Alternatively, rather than setting the existing user profile as the default user, one could also set the default_user_email properly, as the email is asked anyway in the default creation, no?

The problem is that the default_user_email that is defined in the config.json has to correspond to an actual User in the storage backend. That is also why it is not possible to have the user provide a new email for the core.sqlite_zip backend, because that would require adding a new User to the storage, but it is read-only. Therefore, the only solution is to select a user that already exists.

I assume people might still want to create separate, temporary profiles when exploring read-only archives if they already have a main profile? Suddenly having some user from the archive as the default might be confusing.

Note that this is a profile specific setting, not a global one. Each profile has to define its own default user. So selecting a default user for a new temporary profile to browse an archive, will not affect any other profiles.

Is the problem with the default user only eg in the cmdline?

No, this is baked into the entire API. All code expects that a default user is always defined.

It would be possible to rethink how default users are used, and just users in general, but that would be a big redesign. This isn't necessary to just fix the specific problem of the core.sqlite_zip backends I would say