Make command orion db setup ask for right arguments based on storage backend.
#586
Conversation
src/orion/core/cli/db/setup.py
Outdated
| config = {"database": {"type": _type, "name": name, "host": host}} | ||
| # Get database type. | ||
| while True: | ||
| _type = ( |
There was a problem hiding this comment.
I think it would be great to push this loop into ask_question with a choice argument. If there is no choice, the loop breaks directly, otherwise it ensures the user input is valid. What do you think?
src/orion/core/cli/db/setup.py
Outdated
| db_class = Database.types[Database.typenames.index(_type)] | ||
| db_args = db_class.get_arguments() | ||
| arg_vals = {} | ||
| for arg_name, default_value in db_args.items(): |
There was a problem hiding this comment.
I would sort the arguments here to make sure the order is deterministic.
| for arg_name, default_value in db_args.items(): | |
| for arg_name, default_value in sorted(db_args.items()): |
| @@ -330,3 +330,13 @@ def _sanitize_attrs(self): | |||
| self.options["authSource"] = settings["options"].get( | |||
| "authsource", self.name | |||
| ) | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
We could rename this defaults instead and use it inside __init__ to make sure the defaults are coherent. Otherwise we are starting to duplicate the defaults and they may diverge. (defaults here vs defaults in orion.config.storage.database)
There was a problem hiding this comment.
I didn't understand here, what should be renamed to defaults ?
There was a problem hiding this comment.
Oh sorry, the name of the class method, get_defaults.
There was a problem hiding this comment.
Ok ! Renamed get_arguments() to get_defaults() (in derived classes, too).
How should I use it inside __init__ ?
There was a problem hiding this comment.
Inside the init, if a value is set to None or is an empty string, replace with value in get_defaults. Also you will need to use the default values as currently set in config here: https://github.com/Epistimio/orion/blob/develop/src/orion/core/__init__.py#L91. This way we harmonize the defaults.
See @abergeron 's PR for the defaults on PickledDB: https://github.com/Epistimio/orion/pull/585/files#diff-f4fa1b61f95de078bc5673f3d914867b67cef706344b651be8ee5cbeb2d17ad7R102
…nal list) and `ignore_case` (bool, default False). - choice: a list of expected value to check user answer against. - ignore_case: a boolean to tell if checking against choice should be case-insensitive.
src/orion/core/utils/terminal.py
Outdated
| break | ||
| if answer in choice or (ignore_case and answer.lower() in choice): | ||
| break | ||
| print("Unexpected value {}\n".format(answer)) |
There was a problem hiding this comment.
I would add here 'must be one of {}' and then list the valid choices.
…valid database is entered in `test_invalid_database`.
…d as None. Update MongoDB's get_defaults() with database name as "orion".
Description
Update command line
orion db setupto ask for arguments depending of storage backend typed in first question.To automate retrieval of storage backend arguments, a class method
get_arguments()is added to base classAbstractDB.Fixes #573
Checklist
Tests
$ tox -e py38; replace38by your Python version if necessary)Quality
$ tox -e lint)