Skip to content

Add/improve documentation for MLflow gc command#127

Open
dbczumar wants to merge 24 commits into
criteo-forks:hard-delete-runfrom
dbczumar:hard-delete-run
Open

Add/improve documentation for MLflow gc command#127
dbczumar wants to merge 24 commits into
criteo-forks:hard-delete-runfrom
dbczumar:hard-delete-run

Conversation

@dbczumar

Copy link
Copy Markdown

What changes are proposed in this pull request?

This PR makes a couple of small tweaks to the mlflow gc CLI documentation and adds an entry to the MLflow Tracking RST documentation to improve discoverability and clarify existing deletion semantics.

How is this patch tested?

(Details)

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release notes for MLflow users.

(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)

What component(s) does this PR affect?

  • UI
  • CLI
  • API
  • REST-API
  • Examples
  • Docs
  • Tracking
  • Projects
  • Artifacts
  • Models
  • Scoring
  • Serving
  • R
  • Java
  • Python

How should the PR be classified in the release notes? Choose one:

  • rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
  • rn/feature - A new user-facing feature worth mentioning in the release notes
  • rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
  • rn/documentation - A user-facing documentation change worth mentioning in the release notes

andychow-db and others added 20 commits February 11, 2020 09:53
Updated error message to improve readability and customer experience.
The current implementation of autolog for the XGBoost and LightGBM flavors creates new figures when xxx.train is called. If a user calls it multiple times, the following warning is raised. This commit adds plt.close(fig) to close logged figures and remove the warning.
…w#2324)

* Add search input to experiment list

Added search input field to experiment list view.

* Filter experiment list based on input from search field

Added filtering functionality.
When using the experiment search field, only experiments that contain
the searched value will be shown.

* Add unit tests

Added unit tests, testing the added search functionality.

* Format Fix

* Align with naming conventions

* Update focus style of input field

Changed focus style of search box from red to blue since red often indicates an error.

* Update default experiment selection & minor refactoring

* clean, format

Co-authored-by: Richard Zang <Zangr@users.noreply.github.com>
…er attributes change (mlflow#2408)

Fixes issue where metric plot's zoom state would reset when line smoothness or other plot attributes were modified.
…lflow#2348)

* Add delete and update experiment endpoints to service

Added delete and update experiment endpoints to existing
MlflowService implementation. These operations were already supported by
MLflows REST API. For more information, see:
https://www.mlflow.org/docs/latest/rest-api.html
Added corresponding AJAX calls.

* Add edit and delete button to experiment list

Added edit and delete button to experiment list view.
The buttons do not have any functionality associated with them yet.

* Add delete experiment functionality

Added functionality to delete experiments.
When clicking on the delete button in the experiment list,
 confirmation modal is opened.

* Refactor and generalize modal to rename runs

Updated modal to rename runs such that it is more generic.
In particular, updated the displayed form.
As such, this component can be used for renaming experiments.

* Update delete experiment modal

Updated delete experiment modal such that the passed experimentId is
a number instead of a string. Updated ExperimentListView accordingly.

* Add rename experiment functionality

Added functionality to rename experiments.
When clicking on the edit button in the experiment list,
 a modal is opened asking for a new experiment name.

* Fix rename experiment functionality

Fixed a minor issue which caused the experiment name to not be updated.

* Refresh experiment list after deletion

Updated experiment deletion functionality such that
the shown experiment list is refreshed/updated after
performing the deletion.
To do so, the behavior of experimentById reducer had
to be modified.
The previous implementation kept deleted experiments in the state
until the page was refreshed. This could also be observed when
deleting an experiment via the CLI since
the deleted experiment would remain in the UI until refreshing.

* Fix existing unit tests

* Add unit test

Added unit tests testing that the delete and rename
experiment modals are opened after clicking the
respective links in the experiment list.

* Update styles of rename experiment modal

* Disable delete experiment link for active experiment

Updated the experiment list in such a way that
the "Delete Experiment" option is disabled
for the currently active/selected experiment.

* Fix linting issue

* Migrate renaming modals to antd

Based on the following input of the created PR:
mlflow#2348 (comment)
Updated rename modals to use antd modals and forms instead of ReactModal and Formik.

* Add focus and initialValue to input in rename modals

Updated RenameFormView used in rename modals.
The input of the modal will be focused and selected after (re)opening the modal.
The input of the modal will contain the respective name of the experiment/run as the initial value
after re(opening) the modal.

* Create generic input modal & refactor rename modals

Refactored rename modals.
Since the RenameExperimentModal and RenameRunModal had a lot in common,
I put most code in a generic modal component.
This generic modal component is used by the rename modals.

* Migrate ConfirmModal to antd

Based on the following input of the created PR:
mlflow#2348 (comment)
Updated the generic ConfirmModal to use antd modals.

* Update "Delete" icon

* Fix error handling of generic modal

Fixed error handling of generic modal.
Before this change, the error modal was not correctly displayed.

* Align with naming conventions & use wrapped dispatch functions explicitly

Incorporated feedback from PR.
Aligned with naming conventions (callbacks, variables...)
Moved follow-up calls from deleteExperimentApi and deleteRunApi up to be explicitly invoked
as a follow up in the then blocks.
Updated code to use wrapped dispatch functions explicitly.

* Add basic rendering test for InputFormView

* Update GenericInputModal

Refactored GenericInputModal, making the component more generic.
1. A form component can now be passed to the modal as a render prop
2. The entire values object (from the form) is now passed to the submit callback.
The caller can thus decide what to do with the values object.

* Align with naming conventions inside basic rendering test

* Rename InputFormView to RenameForm

* Allow users to delete the current experiment

Updated the experiment deletion behavior.
Users can now delete the currently selected experiment.
As a consequence, they are re-routed to the root which then picks the next active experiment to show.

* Add "Create Experiment" functionality

Added functionality to create experiments from the UI.
Added "Create Experiment" button based on mockup in PR mlflow#2348.
Added modal and form that handles experiment creation.
Added basic unit tests.

* Route user to new experiment page after creation

Updated the behavior when creating an experiment.
After creation, the user will be redirected to the newly created experiment page.
As such, they don not need to manually find the new experiment in the list

* Add getExperimentByName endpoint to service

Added getExperimentByName to MLflowService.
https://www.mlflow.org/docs/latest/rest-api.html#get-experiment-by-name

* Update error handling & validation checks

Unified modal error handling based on the discussion in PR mlflow#2348.
Updated Rename/Create experiment modals to check whether the experiment name is already in use.
Leveraged async-validator to do the validation against the
current experiment name in the form input.
Added debouncing to the validation trigger to prevent excessive calls.
Other errors, e.g. Internal Server Errors, are now displayed in a message instead of a error dialog.

* Update debounced validator usage & use explicit exports

Updated debounced validator. Before, a new instance of the validator function was created per render.
Bound debounce outside the render.
Updated some components to use explicit exports.

* Add unit test for experimentNameValidator

* Add artifact location to CreateExperimentModal

Updated CreateExperimentModal. A user can now specify an artifact location.
Creating an experiment now matches the overall create_experiment signature

* Update named exports & update lodash import

* Update updateExperimentApi

Moved follow-up getExperimentApi call to the rename modal layer.

* Fix NEP error after experiment creation

Updated the experiment creation behavior.
Before, the sequence of the performed calls after a user created an experiment was not guaranteed.
As such, the experiment list was sometimes not refreshed before the rerouting happened which led to an error in the UI.

* Update formatting

* Update UUID handling in CreateExperimentModal

* Update experiment creation to use async/await pattern
Announces deprecation of Python 2 support, to be dropped entirely from MLflow in a future release.
* Updated model registry protos.

* Changes related to new registry apis.

* nit

* nit

* Fixed tests.

* nit.

* Fixed python 2 tests (dicts are unordered).

* Minor fix, added a test.

* Added tests for client compatibility layer.

* minor fixes

* Addressed review comments.

* nit

Co-authored-by: Mani Parkhe <mani@databricks.com>
Fixes a bug introduced in mlflow#2408, namely modifying handleLayoutChange in MetricsPlotPanel to update dragmode of the plot.
* Update R client to call record logged model metadata with tracking server.

* fix

* Ran roxygen to update R docs.

* Updated R NAMESPACE to address cran check complaint.

* test fix

* fix test

* fix test

* Addressed review comment + removed forgotten debug string from earlier.
Metric plot config is now saved in the URL to simplify sharing plots & collaborating
Comment thread docs/source/tracking.rst
Deletion Behavior
~~~~~~~~~~~~~~~~~
In order to allow MLflow Runs to be restored, Run metadata and artifacts are not automatically removed
from the backend store or artifact store when a Run is deleted. The :ref:`mlflow gc <cli>` CLI is provided

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from the backend store or artifact store when a Run is deleted. The :ref:`mlflow gc <cli>` CLI is provided
from the backend store or artifact store when a Run is deleted. The :ref:`mlflow gc <backend_store_uri>` CLI is provided

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-henri This is RST syntax; the bracketed code creates a link to the MLflow CLI docs page; backend_store_uri is not a valid link.

@t-henri

t-henri commented Feb 28, 2020

Copy link
Copy Markdown

Hello @dbczumar thanks for improving the documentation, I made a small recommandation for it.

t-henri and others added 3 commits February 28, 2020 11:15
* Added hard delete run in sql alchemy store. Added delete artifacts in hdfs store. Added hard delete run endpoint and function in client.

* Fixed delete in hdfs artifact repo. Fixed hard delete endpoint.

* Add support for file store

* Artifacts are deleted on client side when using hard-delete run cli

* Added support for local_artifact_repo

* Fix linter

* Added test for fluent api

* Fix tests

* Added not implemented yet exception for artifact stores other than hdfs and local

* Removed hard delete run endpoint and exposition in client.py

* Added gc command line

* Fix tests

* Remove hard delete runs command line

* Add method _get_deleted_runs in sql alchemy store

* Added test for _get_deleted_runs in sql alchemy store

* Fix linter

* Fix linter

* Added comments to _hard_delete_run and to the gc cli.

* Add implementation of get_deleted_runs for file_store

* delete_artifacts now take a relative path. get_deleted_runs in sql alchemry store returns a list of string. Changes to comments.

* Fix linter

* Fix tests

* Added end to end test for mlflow gc command line

* Fix windows tests

* Fix hdfs_artifact_repo tests on windows

* Fix test_cli for python 2

* Fix linter

* Fix python 2 tests

* Fix python 2 tests

* Retrigeer tests becaused of failed R tests

* Changed urllib import to use six.moves
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants