Migrating configuration options to AuthInfo and redesigning the interface #3663
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.
I am creating this PR mainly as a discussion place for the design.
This work originally started to fix #3630 (and #3541). However, while working on it, I found a number of design issues that might benefit for a general redesign.
This code shouldn't be merged yet - it starts some moving around, but after further discussions with @muhrin and @sphuber we might want to change properly the current interaction between transport/AuthInfo/scheduler/... classes (and click).
Here are some points after the discussion - once we agree, we can start doing the actual work applying these decisions.
Design issues:
AuthInfo
it's actually aComputerConfig
object. We don't necessarily need to rename it, but we should think to it that way (all the configuration for a given computer and aiida user pair).auth_params
, that are just piped through to the transport to generate a new instance.__init__
of the transport, and then just set in an internal variable to then return it back when asked. Since there is no functionality, these variables should never go or be known to the Transport (the transport just knows how to open a transport, the caller is in charge of knowing how often this can be done).verdi computer configure xxx
verdi computer configure xxx
.aiida.cmdline.computer.configure
and generate also those from theaiida.transports
entry point (this was already partially happening)._common_auth_options
floating around, but we should have an object that knows about this schema (in a non-click-specific way). Ideally this should be in AuthInfo. The problem is, however, that this means that at import time we need to importaiida.orm
(to get the AuthInfo and generate the click decorators toverdi computer configure xxx
) and this is slow. We need to decide if we want to solve the speed issue first, and use theAuthInfo
object as the main class coordinating all this, or have another class (e.g.aiida.common.computerconfig.ComputerConfig
) that does not know about the backend.AuthInfo
would just wrap it and provide a "proxy" for the relevant methods, but the logic will be mostly in this internal class, from which the CLI can get the schema information (currently_common_auth_options
) without loadingaiida.orm
.ComputerConfig
know about the user and the computer? If so, then it should know about them without needing to go to the orm/DB, otherwise it defeats its purpose. Maybe it just needs to contain the content. However, it needs to know at least the TransportClass it is bound to (maybe in the future also the scheduler?)Other issues: