Add delete function to API #662
Conversation
Remove an artifact from artifact store as well as removing value from both database and pickle store if no other artifacts refer to it.
A KeyError can be raised instead when the pickle file is not found at the expected location.
There might be a bug with artifact versions so the tests with hardcoded versions are xfailed.
Allow user to pass in all as a string into the version parameter of delete to delete all versions of an artifact.
* Fixing pytorch dependencies for integration tests * Fixing comments for pytorch integration tests
There was a problem hiding this comment.
I just had one Q about artifact saving behavior on the same node, otherwise looks great. cc @lionsardesai
| @@ -94,7 +94,11 @@ def save(reference: object, name: str) -> LineaArtifact: | |||
| node_id=value_node_id, execution_id=execution_id | |||
| ): | |||
| # can raise ArtifactSaveException | |||
|
|
|||
| # pickles value of artifact and saves to filesystem | |||
There was a problem hiding this comment.
We prob should have renamed the function name since it's not db but rather file path
|
|
||
| def _try_delete_from_db(pickled_path: Path) -> None: | ||
| if pickled_path.exists(): | ||
| pickled_path.unlink() |
There was a problem hiding this comment.
| @@ -470,6 +470,24 @@ def node_value_in_db( | |||
| .exists() | |||
| ).scalar() | |||
|
|
|||
| def number_of_artifacts_per_node( | |||
There was a problem hiding this comment.
Do we know whether a new pickle gets created when the .save is called on the same node multiple times?
| self, artifact_name: str, version: Union[int, str] = None | ||
| ): | ||
| """ | ||
| Deletes the most recent artifact with a certain name. |
There was a problem hiding this comment.
The description doesn't include a reference to "all" and "latest"?
I don't really like the design using strings but it's fine for now.
tests/integration/test_slice.py
Outdated
| @@ -49,7 +49,9 @@ class Environment: | |||
| pip=["gym[atari]"], | |||
| ), | |||
| "pytorch": lambda: Environment( | |||
There was a problem hiding this comment.
weird that the diff is showing up... maybe you haven't rebased yet with main (I thought this was merged already)?
|
fixes #659 |
Remove an artifact from artifact store as well as
removing value from both database and pickle store if
no other artifacts refer to it.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?