Replace --delete with --delete-destination-extra in transfer commands (…

…#1037)

* Added new --delete-destination-extra flag to transfer commands
* Added deprecation warning to --delete flag
* Updated --delete flag to be hidden

changelog.d/20241015_111820_max.tuecke_sc_35029_deprecate_delete_flag.md

@@ -0,0 +1,5 @@
+Added
+~~~~~
+
+- Added new ``--delete-destination-extra`` flag to ``globus timer create transfer`` and ``globus transfer`` that mirrors the existing ``--delete`` flags' behaviors. (:pr:`1037`)
+- Added deprecation warning to the old ``--delete`` flag. (:pr:`1037`)

src/globus_cli/commands/timer/create/transfer.py

@@ -102,11 +102,22 @@
     "--delete",
     is_flag=True,
     default=False,
+    hidden=True,
     help=(
         "Delete any files in the destination directory not contained in the source. "
         'This results in "directory mirroring." Only valid on recursive transfers.'
     ),
 )
+@click.option(
+    "--delete-destination-extra",
+    is_flag=True,
+    default=False,
+    help=(
+        "Delete any files in the destination directory not contained in the source. "
+        'This results in "directory mirroring." Only valid on recursive transfers.'
+    ),
+)
+@mutex_option_group("--delete", "--delete-destination-extra")
 @LoginManager.requires_login("auth", "timer", "transfer")
 def transfer_command(
     login_manager: LoginManager,
@@ -122,6 +133,7 @@ def transfer_command(
     stop_after_date: datetime.datetime | None,
     stop_after_runs: int | None,
     delete: bool,
+    delete_destination_extra: bool,
     sync_level: t.Literal["exists", "size", "mtime", "checksum"] | None,
     encrypt_data: bool,
     verify_checksum: bool,
@@ -174,6 +186,13 @@ def transfer_command(
     source_endpoint, cmd_source_path = source
     dest_endpoint, cmd_dest_path = destination
 
+    if delete:
+        msg = (
+            "`--delete` has been deprecated and will be removed in a future release. "
+            "Use --delete-destination-extra instead."
+        )
+        click.echo(click.style(msg, fg="yellow"), err=True)
+
     if recursive is not None:
         if batch:
             # avoid 'mutex_option_group', emit a custom error message
@@ -186,6 +205,13 @@ def transfer_command(
         if delete and not recursive:
             msg = "The --delete option cannot be specified with --no-recursion."
             raise click.UsageError(msg)
+        if delete_destination_extra and not recursive:
+            msg = (
+                "The --delete-destination-extra option cannot be specified with "
+                "--no-recursion."
+            )
+            raise click.UsageError(msg)
+
     if (cmd_source_path is None or cmd_dest_path is None) and (not batch):
         raise click.UsageError(
             "transfer requires either SOURCE_PATH and DEST_PATH or --batch"
@@ -278,7 +304,7 @@ def transfer_command(
         encrypt_data=encrypt_data,
         skip_source_errors=skip_source_errors,
         fail_on_quota_errors=fail_on_quota_errors,
-        delete_destination_extra=delete,
+        delete_destination_extra=(delete or delete_destination_extra),
         # mypy can't understand kwargs expansion very well
         **notify,  # type: ignore[arg-type]
     )

src/globus_cli/commands/transfer.py

@@ -140,11 +140,22 @@
     "--delete",
     is_flag=True,
     default=False,
+    hidden=True,
     help=(
         "Delete any files in the destination directory not contained in the source. "
         'This results in "directory mirroring." Only valid on recursive transfers.'
     ),
 )
+@click.option(
+    "--delete-destination-extra",
+    is_flag=True,
+    default=False,
+    help=(
+        "Delete any files in the destination directory not contained in the source. "
+        'This results in "directory mirroring." Only valid on recursive transfers.'
+    ),
+)
+@mutex_option_group("--delete", "--delete-destination-extra")
 @click.option(
     "--external-checksum",
     help=(
@@ -227,6 +238,7 @@ def transfer_command(
     submission_id: str | None,
     dry_run: bool,
     delete: bool,
+    delete_destination_extra: bool,
     deadline: str | None,
     skip_activation_check: bool,
     notify: dict[str, bool],
@@ -326,6 +338,13 @@ def transfer_command(
     source_endpoint, cmd_source_path = source
     dest_endpoint, cmd_dest_path = destination
 
+    if delete:
+        msg = (
+            "`--delete` has been deprecated and will be removed in a future release. "
+            "Use --delete-destination-extra instead."
+        )
+        click.echo(click.style(msg, fg="yellow"), err=True)
+
     # avoid 'mutex_option_group', emit a custom error message
     if recursive is not None and batch:
         option_name = "--recursive" if recursive else "--no-recursive"
@@ -367,7 +386,7 @@ def transfer_command(
         skip_source_errors=skip_source_errors,
         fail_on_quota_errors=fail_on_quota_errors,
         skip_activation_check=skip_activation_check,
-        delete_destination_extra=delete,
+        delete_destination_extra=(delete or delete_destination_extra),
         source_local_user=source_local_user,
         destination_local_user=destination_local_user,
         additional_fields={**perf_opts, **notify},

tests/functional/test_basics.py

@@ -218,3 +218,34 @@ def test_recursive_and_batch_exclusive(run_line, option):
         assert_exit_code=2,
     )
     assert f"You cannot use {option} in addition to --batch" in result.stderr
+
+
+def test_legacy_delete_and_delete_destination_are_mutex(run_line):
+    ep_id = str(uuid.UUID(int=1))
+    result = run_line(
+        [
+            "globus",
+            "transfer",
+            "--delete",
+            "--delete-destination-extra",
+            ep_id,
+            ep_id,
+        ],
+        assert_exit_code=2,
+    )
+    assert "mutually exclusive" in result.stderr
+
+
+def test_legacy_delete_flag_deprecation_warning(run_line):
+    ep_id = str(uuid.UUID(int=1))
+    result = run_line(
+        [
+            "globus",
+            "transfer",
+            "--delete",
+            ep_id,
+            ep_id,
+        ],
+        assert_exit_code=2,
+    )
+    assert "--delete` has been deprecated" in result.stderr

tests/functional/timer/test_transfer_create.py

@@ -303,6 +303,41 @@ def test_stop_conditions_are_mutex(run_line):
     assert "mutually exclusive" in result.stderr
 
 
+def test_legacy_delete_and_delete_destination_are_mutex(run_line):
+    ep_id = str(uuid.UUID(int=1))
+    result = run_line(
+        [
+            "globus",
+            "timer",
+            "create",
+            "transfer",
+            "--delete",
+            "--delete-destination-extra",
+            f"{ep_id}:/foo/",
+            f"{ep_id}:/bar/",
+        ],
+        assert_exit_code=2,
+    )
+    assert "mutually exclusive" in result.stderr
+
+
+def test_timer_creation_legacy_delete_flag_deprecation_warning(run_line):
+    ep_id = str(uuid.UUID(int=1))
+    result = run_line(
+        [
+            "globus",
+            "timer",
+            "create",
+            "transfer",
+            "--delete",
+            f"{ep_id}:/foo/",
+            f"{ep_id}:/bar/",
+        ],
+        assert_exit_code=2,
+    )
+    assert "--delete` has been deprecated" in result.stderr
+
+
 def test_timer_uses_once_schedule_if_stop_after_is_one(run_line, ep_for_timer):
     run_line(
         [
@@ -487,6 +522,16 @@ def test_timer_creation_errors_on_data_access_with_client_creds(
 @pytest.mark.parametrize(
     "deletion_option,recursion_option,expected_error",
     (
+        ("--delete-destination-extra", "--recursive", ""),
+        ("--delete-destination-extra", "", ""),
+        (
+            "--delete-destination-extra",
+            "--no-recursive",
+            (
+                "The --delete-destination-extra option cannot be specified with "
+                "--no-recursion."
+            ),
+        ),
         ("--delete", "--recursive", ""),
         ("--delete", "", ""),
         (