add --include option to globus transfer (#782)

* add --include option to globus transfer

* Reword changelog

Co-authored-by: Kurt McKee <[email protected]>

* Enclose special characters in "

Co-authored-by: Kurt McKee <[email protected]>

* Fix punctuation

Co-authored-by: Kurt McKee <[email protected]>

* Enclose special characters in "

Co-authored-by: Kurt McKee <[email protected]>

* fix variable name in comment

* add Breaking Changes section to changelog

* require globus-sdk 3.19.0

* additional type information and comments

---------

Co-authored-by: Kurt McKee <[email protected]>

Author: aaschaer

changelog.d/20230321_115826_aaschaer_include.md

@@ -0,0 +1,8 @@
+### Enhancements
+
+* Add `--include` option to `globus transfer` allowing ordered overrides of `--exclude` rules.
+
+### Breaking Changes
+
+* The `--exclude` option for `globus transfer` now only applies to files to better
+  support excluding files within a directory structure

setup.py

@@ -45,7 +45,7 @@ def read_readme():
     package_dir={"": "src"},
     python_requires=">=3.7",
     install_requires=[
-        "globus-sdk==3.17.0",
+        "globus-sdk==3.19.0",
         "click>=8.0.0,<9",
         "jmespath==1.0.1",
         "packaging>=17.0",

src/globus_cli/commands/transfer.py

@@ -33,6 +33,12 @@
 
 @command(
     "transfer",
+    # the order of filter_rules determines behavior, so we need to combine
+    # include and exclude options during argument parsing to preserve their ordering
+    opts_to_combine={
+        "include": "filter_rules",
+        "exclude": "filter_rules",
+    },
     short_help="Submit a transfer task (asynchronous)",
     adoc_examples="""Transfer a single file:
 
@@ -160,15 +166,30 @@
     show_default=True,
     help=("Specify an algorithm for --external-checksum or --verify-checksum"),
 )
+@click.option(
+    "--include",
+    multiple=True,
+    show_default=True,
+    expose_value=False,  # this is combined into the filter_rules parameter
+    help=(
+        "Include files found with names that match the given pattern in "
+        'recursive transfers. Pattern may include "*", "?", or "[]" for Unix-style '
+        "globbing. This option can be given multiple times along with "
+        "--exclude to control which files are transferred, with earlier "
+        "options having priority."
+    ),
+)
 @click.option(
     "--exclude",
     multiple=True,
     show_default=True,
+    expose_value=False,  # this is combined into the filter_rules parameter
     help=(
-        "Exclude files and directories found with names that match the given "
-        "pattern in recursive transfers. Pattern may include * ? or [] for "
-        "unix style globbing. Give this option multiple times to exclude "
-        "multiple patterns."
+        "Exclude files found with names that match the given pattern in "
+        'recursive transfers. Pattern may include "*", "?", or "[]" for Unix-style '
+        "globbing. This option can be given multiple times along with "
+        "--include to control which files are transferred, with earlier "
+        "options having priority."
     ),
 )
 @click.option("--perf-cc", type=int, hidden=True)
@@ -189,7 +210,7 @@ def transfer_command(
     external_checksum: str | None,
     skip_source_errors: bool,
     fail_on_quota_errors: bool,
-    exclude: tuple[str, ...],
+    filter_rules: list[tuple[Literal["include", "exclude"], str]],
     label: str | None,
     preserve_timestamp: bool,
     verify_checksum: bool,
@@ -269,6 +290,20 @@ def transfer_command(
     If a transfer fails, CHECKSUM must be used to restart the transfer.
     All other levels can lead to data corruption.
 
+    \b
+    === Include and Exclude
+
+    The `--include` and `--exclude` options are evaluated in order together
+    to determine which files are transferred during recursive transfers.
+    Earlier `--include` and `exclude` options have priority over later such
+    options, with the first option that matches the name of a file being
+    applied. A file that does not match any `--include` or `exclude` options
+    is included by default, making the `--include` option only useful for
+    overriding later `--exclude` options.
+
+    For example, `globus transfer --include *.txt --exclude * ...` will
+    only transfer files ending in .txt found within the directory structure.
+
     {AUTOMATIC_ACTIVATION}
     """
     from globus_cli.services.transfer import add_batch_to_transfer_data, autoactivate
@@ -321,8 +356,9 @@ def transfer_command(
         additional_fields={**perf_opts, **notify},
     )
 
-    for exclude_name in exclude:
-        transfer_data.add_filter_rule(exclude_name)
+    for rule in filter_rules:
+        method, name = rule
+        transfer_data.add_filter_rule(method=method, name=name, type="file")
 
     if batch:
         add_batch_to_transfer_data(
@@ -348,8 +384,10 @@ def transfer_command(
     else:
         has_recursive_items = False
 
-    if exclude and not has_recursive_items:
-        raise click.UsageError("--exclude can only be used with --recursive transfers")
+    if filter_rules and not has_recursive_items:
+        raise click.UsageError(
+            "--include and --exclude can only be used with --recursive transfers"
+        )
 
     if dry_run:
         display(

src/globus_cli/parsing/commands.py

@@ -30,7 +30,27 @@ class GlobusCommand(click.Command):
     adoc generator.
 
     It also automatically runs string formatting on command helptext to allow the
-    inclusion of common strings (e.g. autoactivation help).
+    inclusion of common strings (e.g. autoactivation help) and handles
+    custom argument parsing.
+
+    opts_to_combine is an interface for combining multiple options while preserving
+    their original order. Given a dict of original option names as keys
+    and combined option names as values, options are combined into a list of
+    tuples of the original option name and value. For example:
+
+    @command(
+        ...
+        opts_to_combine={
+            "foo": "foo_bar",
+            "bar": "foo_bar",
+        },
+    @click.option("--foo", multiple=True, expose_value=False)
+    @click.option("--bar", multiple=True, expose_value=False)
+    def example_command(*, foo_bar: list[tuple[Literal["foo", "bar"], Any]]):
+
+        for option in foo_bar:
+            original_option_name, value = option
+
     """
 
     AUTOMATIC_ACTIVATION_HELPTEXT = """=== Automatic Endpoint Activation
@@ -46,6 +66,7 @@ def __init__(self, *args, **kwargs):
         self.globus_disable_opts = kwargs.pop("globus_disable_opts", [])
         self.adoc_exit_status = kwargs.pop("adoc_exit_status", None)
         self.adoc_synopsis = kwargs.pop("adoc_synopsis", None)
+        self.opts_to_combine = kwargs.pop("opts_to_combine", {})
 
         helptext = kwargs.pop("help", None)
         if helptext:
@@ -74,7 +95,28 @@ def parse_args(self, ctx: click.Context, args: list[str]) -> list[str]:
         # args will be consumed, so check it before super()
         had_args = bool(args)
         try:
+            # if we have any opts to be combined in order, do that now
+            if self.opts_to_combine:
+                combined_opts: dict[str, list[tuple[str, str]]] = {
+                    combined_name: [] for combined_name in self.opts_to_combine.values()
+                }
+                parser: click.parser.OptionParser = self.make_parser(ctx)
+                values, _, order = parser.parse_args(args=list(args))
+                # values is a dict of value lists keyed by their option name
+                # in order for that value and order is a list of option names
+                # in the order they were given at the command line
+                # we want a list of (name, value) tuples for multiple options
+                # in the order they were given at the command line
+                for opt in order:
+                    if opt.name and opt.name in self.opts_to_combine:
+                        value = values[opt.name].pop(0)
+                        combined_name = self.opts_to_combine[opt.name]
+                        combined_opts[combined_name].append((opt.name, value))
+
+                ctx.params.update(combined_opts)
+
             return super().parse_args(ctx, args)
+
         except click.MissingParameter as e:
             if not had_args:
                 click.secho(e.format_message(), fg="yellow", err=True)

tests/functional/task/test_task_submit.py

@@ -3,24 +3,47 @@
 from globus_sdk._testing import load_response_set
 
 
-def test_exclude(run_line, go_ep1_id, go_ep2_id):
+def test_filter_rules(run_line, go_ep1_id, go_ep2_id):
     """
-    Submits two --exclude options on a transfer, confirms they show up
-    in --dry-run output
+    Submits two --exclude and two --include options on a transfer, confirms
+    they show up the correct order in --dry-run output
     """
     # put a submission ID and autoactivate response in place
     load_response_set("cli.get_submission_id")
     load_response_set("cli.transfer_activate_success")
 
     result = run_line(
         "globus transfer -F json --dry-run -r "
-        "--exclude *.txt --exclude *.pdf "
+        "--exclude foo --include bar "
+        "--include baz --exclude qux "
         "{}:/ {}:/".format(go_ep1_id, go_ep1_id)
     )
 
     expected_filter_rules = [
-        {"DATA_TYPE": "filter_rule", "method": "exclude", "name": "*.txt"},
-        {"DATA_TYPE": "filter_rule", "method": "exclude", "name": "*.pdf"},
+        {
+            "DATA_TYPE": "filter_rule",
+            "method": "exclude",
+            "name": "foo",
+            "type": "file",
+        },
+        {
+            "DATA_TYPE": "filter_rule",
+            "method": "include",
+            "name": "bar",
+            "type": "file",
+        },
+        {
+            "DATA_TYPE": "filter_rule",
+            "method": "include",
+            "name": "baz",
+            "type": "file",
+        },
+        {
+            "DATA_TYPE": "filter_rule",
+            "method": "exclude",
+            "name": "qux",
+            "type": "file",
+        },
     ]
 
     json_output = json.loads(result.output)
@@ -38,7 +61,10 @@ def test_exlude_recursive(run_line, go_ep1_id, go_ep2_id):
         "globus transfer --exclude *.txt " "{}:/ {}:/".format(go_ep1_id, go_ep1_id),
         assert_exit_code=2,
     )
-    assert "--exclude can only be used with --recursive transfers" in result.stderr
+    assert (
+        "--include and --exclude can only be used with --recursive transfers"
+        in result.stderr
+    )
 
 
 def test_exlude_recursive_batch_stdin(run_line, go_ep1_id, go_ep2_id):
@@ -49,7 +75,10 @@ def test_exlude_recursive_batch_stdin(run_line, go_ep1_id, go_ep2_id):
         stdin="abc /def\n",
         assert_exit_code=2,
     )
-    assert "--exclude can only be used with --recursive transfers" in result.stderr
+    assert (
+        "--include and --exclude can only be used with --recursive transfers"
+        in result.stderr
+    )
 
 
 def test_exlude_recursive_batch_file(run_line, go_ep1_id, go_ep2_id, tmp_path):
@@ -69,4 +98,7 @@ def test_exlude_recursive_batch_file(run_line, go_ep1_id, go_ep2_id, tmp_path):
         ],
         assert_exit_code=2,
     )
-    assert "--exclude can only be used with --recursive transfers" in result.stderr
+    assert (
+        "--include and --exclude can only be used with --recursive transfers"
+        in result.stderr
+    )