Skip to content

Commit

Permalink
Using --force for delete works again (#1377)
Browse files Browse the repository at this point in the history
If a configuration error occurs (say while learning Globus Compute), then the
`delete` codes prevented deletion of the endpoint's configuration directory
-- even if the user specified `--force`.  That's clearly not correct.  If a
user specifies `--force`, then that directory needs to get removed.  This isn't
a 100% solution, but is a pragmatic approach to the actually-found bug: be
forgiving of a parse error because _we're deleting it anyway_.

[sc-28515]
  • Loading branch information
khk-globus committed Dec 1, 2023
1 parent 18fb820 commit 420cc0b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
19 changes: 16 additions & 3 deletions compute_endpoint/globus_compute_endpoint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,13 +593,26 @@ def list_endpoints():
@handle_auth_errors
def delete_endpoint(*, ep_dir: pathlib.Path, force: bool, yes: bool):
"""Deletes an endpoint and its config."""

ep_conf = None
try:
ep_conf = get_config(ep_dir)
except Exception as e:
print(f"({type(e).__name__}) {e}\n")
if not yes:
yes = click.confirm(
f"Failed to read configuration from {ep_dir}/\n"
f" Are you sure you want to delete endpoint <{ep_dir.name}>?",
abort=True,
)

if not yes:
click.confirm(
f"Are you sure you want to delete the endpoint named <{ep_dir.name}>?",
yes = click.confirm(
f"Are you sure you want to delete endpoint <{ep_dir.name}>?",
abort=True,
)

Endpoint.delete_endpoint(ep_dir, get_config(ep_dir), force=force)
Endpoint.delete_endpoint(ep_dir, ep_conf, force=force)


@app.command("self-diagnostic")
Expand Down
14 changes: 14 additions & 0 deletions compute_endpoint/tests/unit/test_cli_behavior.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from globus_compute_endpoint.endpoint.config import Config
from globus_compute_endpoint.endpoint.config.utils import load_config_yaml
from globus_compute_endpoint.endpoint.endpoint import Endpoint
from globus_compute_sdk.sdk.login_manager.tokenstore import ensure_compute_dir
from globus_compute_sdk.sdk.web_client import WebClient
from pyfakefs import fake_filesystem as fakefs
from pytest_mock import MockFixture
Expand Down Expand Up @@ -427,6 +428,19 @@ def test_delete_endpoint(read_config, run_line, mock_cli_state, ep_name):
mock_ep.delete_endpoint.assert_called_once()


@mock.patch("globus_compute_endpoint.endpoint.endpoint.Endpoint.get_funcx_client")
def test_delete_endpoint_with_malformed_config_sc28515(
mock_func, fs, run_line, ep_name
):
compute_dir = ensure_compute_dir()
conf_dir = compute_dir / ep_name
conf_dir.mkdir()
(conf_dir / "config.yaml").write_text("Gobble ty: gook\nnonsense: 1")
assert conf_dir.exists() and conf_dir.is_dir()
run_line(f"delete {ep_name} --yes --force")
assert not conf_dir.exists()


@pytest.mark.parametrize("die_with_parent", [True, False])
@mock.patch("globus_compute_endpoint.cli.get_config")
def test_die_with_parent_detached(
Expand Down

0 comments on commit 420cc0b

Please sign in to comment.