Skip to content

Commit

Permalink
support force remove for model/dataset/runtime/eval
Browse files Browse the repository at this point in the history
  • Loading branch information
tianweidut committed Sep 20, 2022
1 parent 778cab1 commit 00b49ff
Show file tree
Hide file tree
Showing 16 changed files with 94 additions and 34 deletions.
22 changes: 21 additions & 1 deletion client/starwhale/base/bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
DEFAULT_MANIFEST_NAME,
)
from starwhale.base.tag import StandaloneTag
from starwhale.utils.fs import ensure_dir, ensure_file, extract_tar
from starwhale.utils.fs import move_dir, empty_dir, ensure_dir, ensure_file, extract_tar
from starwhale.utils.venv import SUPPORTED_PIP_REQ
from starwhale.utils.error import FileTypeError, NotFoundError, MissingFieldError
from starwhale.utils.config import SWCliConfigMixed
Expand Down Expand Up @@ -242,3 +242,23 @@ def _get_src_walker(
# Notice: if pass [] as exclude_dirs value, walker will failed
_exclude = _exclude or None # type: ignore
return Walker(filter=_filter, exclude_dirs=_exclude)

def _do_remove(self, force: bool = False) -> t.Tuple[bool, str]:
from .store import BaseStorage

store: BaseStorage = self.store # type: ignore

if force:
empty_dir(store.loc)
empty_dir(store.snapshot_workdir)
return True, ""
else:
_ok, _reason = move_dir(store.loc, store.recover_loc, False)
_ok2, _reason2 = True, ""
if store.snapshot_workdir.exists():
_ok2, _reason2 = move_dir(
store.snapshot_workdir,
store.recover_snapshot_workdir,
False,
)
return _ok and _ok2, _reason + _reason2
4 changes: 4 additions & 0 deletions client/starwhale/base/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,7 @@ def get_manifest_by_path(
with TarFS(str(fpath)) as tar:
with tar.open(DEFAULT_MANIFEST_NAME) as f:
return yaml.safe_load(f)

@property
def recover_snapshot_workdir(self) -> Path:
return self._get_recover_snapshot_workdir_for_bundle()
7 changes: 6 additions & 1 deletion client/starwhale/core/dataset/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@ def _info(view: t.Type[DatasetTermView], dataset: str, fullname: bool) -> None:

@dataset_cmd.command("remove")
@click.argument("dataset")
@click.option("-f", "--force", is_flag=True, help="Force to remove dataset")
@click.option(
"-f",
"--force",
is_flag=True,
help="Force to remove dataset, the removed dataset cannot recover",
)
@click.pass_obj
def _remove(view: t.Type[DatasetTermView], dataset: str, force: bool) -> None:
"""
Expand Down
8 changes: 6 additions & 2 deletions client/starwhale/core/dataset/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
)
from starwhale.base.tag import StandaloneTag
from starwhale.base.uri import URI
from starwhale.utils.fs import move_dir, copy_file, ensure_dir
from starwhale.utils.fs import move_dir, copy_file, empty_dir, ensure_dir
from starwhale.base.type import URIType, BundleType, InstanceType
from starwhale.base.cloud import CloudRequestMixed, CloudBundleModelMixin
from starwhale.utils.http import ignore_error
Expand Down Expand Up @@ -196,7 +196,11 @@ def history(

def remove(self, force: bool = False) -> t.Tuple[bool, str]:
# TODO: remove by tag
return move_dir(self.store.snapshot_workdir, self.store.recover_loc, force)
if force:
empty_dir(self.store.snapshot_workdir)
return True, ""
else:
return move_dir(self.store.snapshot_workdir, self.store.recover_loc, False)

def recover(self, force: bool = False) -> t.Tuple[bool, str]:
dest_path = (
Expand Down
7 changes: 6 additions & 1 deletion client/starwhale/core/eval/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ def _run(

@eval_job_cmd.command("remove", help="Remove job")
@click.argument("job")
@click.option("-f", "--force", is_flag=True, help="Force to remove")
@click.option(
"-f",
"--force",
is_flag=True,
help="Force to remove, the removed job cannot recover",
)
def _remove(job: str, force: bool) -> None:
click.confirm("continue to remove?", abort=True)
JobTermView(job).remove(force)
Expand Down
8 changes: 6 additions & 2 deletions client/starwhale/core/eval/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from starwhale.utils import load_yaml
from starwhale.consts import HTTPMethod, DEFAULT_PAGE_IDX, DEFAULT_PAGE_SIZE
from starwhale.base.uri import URI
from starwhale.utils.fs import move_dir
from starwhale.utils.fs import move_dir, empty_dir
from starwhale.api._impl import wrapper
from starwhale.base.type import InstanceType, JobOperationType
from starwhale.base.cloud import CloudRequestMixed
Expand Down Expand Up @@ -262,7 +262,11 @@ def info(
}

def remove(self, force: bool = False) -> t.Tuple[bool, str]:
return move_dir(self.store.loc, self.store.recover_loc, force)
if force:
empty_dir(self.store.loc)
return True, ""
else:
return move_dir(self.store.loc, self.store.recover_loc, False)

def recover(self, force: bool = False) -> t.Tuple[bool, str]:
return move_dir(self.store.recover_loc, self.store.loc, force)
Expand Down
9 changes: 7 additions & 2 deletions client/starwhale/core/model/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,14 @@ def _history(view: t.Type[ModelTermView], model: str, fullname: bool) -> None:

@model_cmd.command("remove", help="Remove model")
@click.argument("model")
@click.option("-f", "--force", is_flag=True, help="Force to remove model")
@click.option(
"-f",
"--force",
is_flag=True,
help="Force to remove model, the removed model cannot recover",
)
def _remove(model: str, force: bool) -> None:
click.confirm("continue to delete?", abort=True)
click.confirm("continue to remove?", abort=True)
ModelTermView(model).remove(force)


Expand Down
8 changes: 1 addition & 7 deletions client/starwhale/core/model/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,7 @@ def history(
return _r

def remove(self, force: bool = False) -> t.Tuple[bool, str]:
_ok, _reason = move_dir(self.store.loc, self.store.recover_loc, force)
_ok2, _reason2 = True, ""
if self.store.snapshot_workdir.exists():
_ok2, _reason2 = move_dir(
self.store.snapshot_workdir, self.store.recover_snapshot_workdir, force
)
return _ok and _ok2, _reason + _reason2
return self._do_remove(force)

def recover(self, force: bool = False) -> t.Tuple[bool, str]:
# TODO: support short version to recover, today only support full-version
Expand Down
4 changes: 0 additions & 4 deletions client/starwhale/core/model/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,3 @@ def manifest_path(self) -> Path:
@property
def snapshot_workdir(self) -> Path:
return self._get_snapshot_workdir_for_bundle()

@property
def recover_snapshot_workdir(self) -> Path:
return self._get_recover_snapshot_workdir_for_bundle()
7 changes: 6 additions & 1 deletion client/starwhale/core/runtime/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,12 @@ def _build(

@runtime_cmd.command("remove")
@click.argument("runtime")
@click.option("-f", "--force", is_flag=True, help="Force to remove runtime")
@click.option(
"-f",
"--force",
is_flag=True,
help="Force to remove runtime, the removed runtime cannot recover",
)
def _remove(runtime: str, force: bool) -> None:
"""
Remove runtime
Expand Down
8 changes: 1 addition & 7 deletions client/starwhale/core/runtime/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,7 @@ def remove_tags(self, tags: t.List[str], quiet: bool = False) -> None:
self.tag.remove(tags, quiet)

def remove(self, force: bool = False) -> t.Tuple[bool, str]:
_ok, _reason = move_dir(self.store.loc, self.store.recover_loc, force)
_ok2, _reason2 = True, ""
if self.store.snapshot_workdir.exists():
_ok2, _reason2 = move_dir(
self.store.snapshot_workdir, self.store.recover_snapshot_workdir, force
)
return _ok and _ok2, _reason + _reason2
return self._do_remove(force)

def recover(self, force: bool = False) -> t.Tuple[bool, str]:
# TODO: support short version to recover, today only support full-version
Expand Down
4 changes: 0 additions & 4 deletions client/starwhale/core/runtime/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ def recover_loc(self) -> Path:
def snapshot_workdir(self) -> Path:
return self._get_snapshot_workdir_for_bundle()

@property
def recover_snapshot_workdir(self) -> Path:
return self._get_recover_snapshot_workdir_for_bundle()

@property
def export_dir(self) -> Path:
return self.snapshot_workdir / "export"
Expand Down
6 changes: 5 additions & 1 deletion client/tests/core/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_build_workflow(self, m_import: MagicMock, m_copy_fs: MagicMock) -> None
f"mnist/version/{build_version}", expected_type=URIType.DATASET
)
sd = StandaloneDataset(dataset_uri)
_ok, _ = sd.remove(True)
_ok, _ = sd.remove(False)
assert _ok

_list, _ = StandaloneDataset.list(URI(""))
Expand All @@ -135,6 +135,10 @@ def test_build_workflow(self, m_import: MagicMock, m_copy_fs: MagicMock) -> None
DatasetTermView(fname).recover()
DatasetTermView.list()

sd.remove(True)
_list, _ = StandaloneDataset.list(URI(""))
assert len(_list[name]) == 0

DatasetTermView.build(workdir, "self")

# make sure tmp dir is empty
Expand Down
11 changes: 11 additions & 0 deletions client/tests/core/test_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ def test_remove(self):
/ self.job_name
).exists()

job.remove(True)
assert not os.path.exists(self.job_dir)
assert not (
Path(self.root)
/ "self"
/ URIType.EVALUATION
/ RECOVER_DIRNAME
/ self.job_name[:2]
/ self.job_name
).exists()

@patch("starwhale.core.eval.model.subprocess.check_output")
@patch("starwhale.core.eval.model.check_call")
def test_actions(self, m_call: MagicMock, m_call_output: MagicMock):
Expand Down
7 changes: 6 additions & 1 deletion client/tests/core/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def test_build_workflow(self, m_copy_fs: MagicMock, m_copy_file: MagicMock) -> N

model_uri = URI(f"{name}/version/{build_version}", expected_type=URIType.MODEL)
sd = StandaloneModel(model_uri)
_ok, _ = sd.remove(True)
_ok, _ = sd.remove(False)
assert _ok

_list, _ = StandaloneModel.list(URI(""))
Expand All @@ -142,6 +142,11 @@ def test_build_workflow(self, m_copy_fs: MagicMock, m_copy_file: MagicMock) -> N
ModelTermView.list(show_removed=True)
ModelTermView.list()

_ok, _ = sd.remove(True)
assert _ok
_list, _ = StandaloneModel.list(URI(""))
assert len(_list[name]) == 0

ModelTermView.build(workdir, "self")

@Mocker()
Expand Down
8 changes: 8 additions & 0 deletions client/tests/core/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,14 @@ def test_build_venv(
assert not os.path.exists(recover_snapshot_path)
assert os.path.exists(swrt_snapshot_path)

rtv = RuntimeTermView(f"{name}/version/{build_version}")
ok, _ = rtv.remove(True)
assert ok
assert not os.path.exists(recover_path)
assert not os.path.exists(swrt_path)
assert not os.path.exists(recover_snapshot_path)
assert not os.path.exists(swrt_snapshot_path)

@patch("starwhale.utils.venv.get_user_runtime_python_bin")
@patch("starwhale.utils.venv.is_venv")
@patch("starwhale.utils.venv.is_conda")
Expand Down

0 comments on commit 00b49ff

Please sign in to comment.