Allow MLflow run names without asset key #1
Conversation
|
Thoughts from @nicholasjng and me:
No idea, but we can take time together tomorrow.
No. It is not a bad idea. Repeatedly overwriting the last run keeps the mlflow run logs lean and in the way we set it up with the start up script. We only show the MLflow page once. If we do that after we execute the workflow it is also not surprising when the run already exists.
If we drop the Dagster run id, it does not make sense to log the asset key. Plus, the logged names can be (are?) different if we prefix them with e.g. In our story we will not have time to dive into details like the asset key. Thus we'd advocate for dropping |
|
|
||
| if run_name_prefix is not None: | ||
| run_name = f"{run_name_prefix}-{run_name}" | ||
| parts.append(run_name_prefix) |
There was a problem hiding this comment.
We should either name this suffix, or actually use it as a prefix
| dagster_run_id = get_run_id(context, short=True) | ||
|
|
||
| run_name = f"{asset_key}-{dagster_run_id}" | ||
| if run_name_prefix is None: |
There was a problem hiding this comment.
Should we just call the param prefix? The method name _get_run_name_from_context makes it clear that the prefix concerns a run name
Co-authored-by: Nicholas Junge <[email protected]>
Open questions:
AssetExecutionContextin a test case.use_asset_run_key=False, the asset key tags on the MLflow run are incomplete (since every Dagster op/asset replaces the previous value). Should we drop that tag entirely? Does any other code depend on this feature currently?