Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the mlflow.yml location dependent of the ConfigLoader configuration #159

Closed
Galileo-Galilei opened this issue Jan 25, 2021 · 1 comment · Fixed by #162
Closed

Make the mlflow.yml location dependent of the ConfigLoader configuration #159

Galileo-Galilei opened this issue Jan 25, 2021 · 1 comment · Fixed by #162
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Galileo-Galilei
Copy link
Owner

Description

Currently, the kedro mlflow init command creates the mlflow.yml configuration file to the hardcoded location conf/base/mlflow.yml. Some users may customize their ConfigLoader and do not have a conf/base folder. In general, this tightens the plugin to the kedro template, which is a bad practice (it decreases modularity and extensibility).

It would be better to create the configuration file based on ConfigLoader configuration.

Context

I used kedro-mlflow in a custom template hich does not have a conf/base folder, which (unnecessarily) prevents using the plugin.

Possible Implementation

Maybe create the file at context.config_loader.conf_paths[0]?

@Galileo-Galilei Galileo-Galilei self-assigned this Jan 25, 2021
@Galileo-Galilei Galileo-Galilei added the enhancement New feature or request label Jan 25, 2021
@Galileo-Galilei Galileo-Galilei added this to To do in Ongoing development via automation Jan 25, 2021
@Galileo-Galilei Galileo-Galilei added this to the Release 0.5.0 milestone Jan 25, 2021
@Galileo-Galilei
Copy link
Owner Author

Galileo-Galilei commented Feb 7, 2021

Here are the points at stake here:

  • if the code runs in kedro without kedro-mlflow, it must run with kedro-mlflow. The rationale on displaying dynamically the CLI commands to make projects commands available only after the user ran kedro mlflow init adds some extra logic which breaks this principle. In fact, if someone makes some customisation to the configloader or the configuration loading logic, it is too restrictive to assume that the mlflow.yml exists in conf/base folder. ->Decision: remove dynmaic display and always display all projects commands at the root of a kedro project.
  • The CLI should enable to specify an environment where to create mlflow.yml (and not assume it is in necessarily in base) -> add an extra argument to kedro mlflow init
  • Since commands will now always be available (even in uninitialized projects), an informative error should be displayed if the configuration does not exist. -> an an extra error message in get_mlflow_config
  • The mlflow.yml should be created by default in conf/local instead of conf/base, because most of this configuration is expected to be overridden.

Galileo-Galilei added a commit that referenced this issue Feb 7, 2021
Galileo-Galilei added a commit that referenced this issue Feb 7, 2021
@Galileo-Galilei Galileo-Galilei moved this from To do to Pending review in Ongoing development Feb 14, 2021
Ongoing development automation moved this from Pending review to Done Feb 15, 2021
Galileo-Galilei added a commit that referenced this issue Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
1 participant