feature: add filtering of instructions on the exec command (#210)

## Summary:

 - Add a `--exclude/-e` and `--include/-i` flags on the exec command to filter instructions. The filters are being validated then forwarded to SCLE where the filtering is done: https://github.com/strateos/lab/pull/1527
 - Minor: use email address to specify who sent the payload.

Require: https://github.com/strateos/lab/pull/1526 to be merged in lab as well
### Usage:
```
cat autoprotocol.json | \
transcriptic exec -a lab.core.strateos.com/haven/wctest -w wc4-mcx1 -e 1-9 -i 7 -e op:provision
```
This command will first filter out instruction from index 1 to 9 (`-e 1-9`) and the provisions (`-e op:provision`). Then add back the instruction at index 7 (`-i 7`).

The order of `-i/-e` don't matter, it is first removing all the `-e`, then adding back all the `-i`.

### Filters

Possible filter:
 - single index: 123
 - range: 123-456
 - key:value in the instruction: `op:provision`, or `x_human!:false`. The ! means that if the key (x_human) is not in the instruction, then the filter matches. Otherwise, without the '!' a missing key means that there is no match.

Filter example:
  - { "op": "seal", "x_human": false } and { "op": "seal" } match `x_human!:false`
  - { "op": "seal" } does not match`x_human:true`, nor `x_human:false`

### Test plan
Add unit tests

#### Manual testing

```
internal_protocols/clients/sd2/current_protocols/growth_curve [master*]$ transcriptic preview GrowthCurve | \
transcriptic exec -a lab.core.strateos.com/haven/wctest -w wc4-mcx1 -e x_human:true
Sending request to http://13.52.223.111:9000/testRun
Success. View http://lab.core.strateos.com/haven/wctest/dashboard to see the scheduling outcome.
```
or
```
internal_protocols/clients/sd2/current_protocols/growth_curve [master*]$ transcriptic preview GrowthCurve | \
transcriptic exec -a lab.core.strateos.com/haven/wctest -w wc4-mcx1 -e op:provision
Sending request to http://13.52.223.111:9000/testRun
Success. View http://lab.core.strateos.com/haven/wctest/dashboard to see the scheduling outcome.
```

Author: GregoryEssertel

test/commands/exec_test.py

@@ -13,8 +13,11 @@ def queue_test_success_res(sessionId="testSessionId"):
     return {"success": True, "sessionId": sessionId}
 
 
+fake_valid_URL = "something.bar.foo"
+
+
 def app_config_res():
-    return {"hostManifest": {"lab": {"workcell": {"url": "something.bar.foo"}}}}
+    return {"hostManifest": {"lab": {"workcell": {"url": fake_valid_URL}}}}
 
 
 def mock_api_endpoint():
@@ -30,7 +33,15 @@ def ap_file(tmpdir_factory):
     """Make a temp autoprotocol file"""
     path = tmpdir_factory.mktemp("foo").join("ap.json")
     with open(str(path), "w") as f:
-        f.write("{}")  # any valid json works
+        payload = {
+            "instructions": [
+                {"op": "provision", "x_human": True},
+                {"op": "uncover"},
+                {"op": "spin"},
+                {"op": "cover"},
+            ]
+        }
+        f.write(json.dumps(payload))
     return path
 
 
@@ -94,7 +105,7 @@ def mockpost(*args, **kwargs):
     result = cli_test_runner.invoke(
         cli, ["exec", str(ap_file), "-a", mock_api_endpoint()]
     )
-    assert result.exit_code == 0
+    assert result.exit_code != 0
     assert "Error: " in result.stderr
 
 
@@ -160,5 +171,30 @@ def test_too_many_workcell_definition_arguments(cli_test_runner, monkeypatch, ap
         cli,
         ["exec", str(ap_file), "-a", mock_api_endpoint(), "-s", "anthing", "-w", "wc0"],
     )
-    assert result.exit_code == 0
+    assert result.exit_code != 0
     assert "Error: --workcell-id, --session-id are mutually exclusive." in result.stderr
+
+
+def test_invalid_filters(cli_test_runner, monkeypatch, ap_file):
+    invalid_command = [
+        "-e",
+        "10",
+        "-i",
+        "10",
+        "-e",
+        "2-1",
+        "-i",
+        "3-6",
+        "-e",
+        "anything",
+    ]
+    invalid_filters = set(filter(lambda v: not v.startswith("-"), invalid_command))
+    result = cli_test_runner.invoke(
+        cli,
+        ["exec", str(ap_file), "-a", mock_api_endpoint()] + invalid_command,
+    )
+    assert result.exit_code != 0
+    assert "Error: invalid filters" in result.stderr
+    assert "(number of instructions: 4)" in result.stderr
+    for inv in invalid_filters:
+        assert inv in result.stderr

transcriptic/cli.py

@@ -690,6 +690,20 @@ def format_cmd(manifest):
     help="If set, the time constraints will be considered only a suggestion.",
     is_flag=True,
 )
+@click.option(
+    "--exclude",
+    "-e",
+    metavar="FILTER",
+    help="Remove those instructions from the payload. e.g.: 0, 0-5, x_human, op:povision, x_human!:false",
+    multiple=True,
+)
+@click.option(
+    "--include",
+    "-i",
+    metavar="FILTER",
+    help="Add those instructions to the payload after the --exclude has been applied.",
+    multiple=True,
+)
 @click.option(
     "--partition-group-size",
     type=click.INT,
@@ -707,7 +721,9 @@ def format_cmd(manifest):
     default=None,
     help="The device id to use as a swap space when partitioning.",
 )
+@click.pass_context
 def execute(
+    ctx,
     autoprotocol,
     api,
     no_redirect,
@@ -718,6 +734,8 @@ def execute(
     schedule_at,
     schedule_delay,
     time_constraints_are_suggestion,
+    exclude,
+    include,
     partition_group_size,
     partition_horizon,
     partitioning_swap_device_id,
@@ -734,7 +752,10 @@ def execute(
         schedule_at,
         schedule_delay,
         time_constraints_are_suggestion,
+        exclude,
+        include,
         partition_group_size,
         partition_horizon,
         partitioning_swap_device_id,
+        ctx.obj.api.email,
     )

transcriptic/commands.py

@@ -1474,6 +1474,28 @@ def run_protocol(api, manifest, protocol, inputs, view=False, dye_test=False):
             return
 
 
+def validate_filter(filters, number_of_instructions):
+    invalid_filters = set()
+
+    for arg in filters:
+        if arg.isdigit():
+            idx = int(arg)
+            if 0 > idx or idx >= number_of_instructions:
+                invalid_filters.add(arg)
+        elif re.match(r"\d+-\d+", arg):
+            [s, e] = [int(v) for v in arg.split("-")]
+            if s > e or number_of_instructions <= e:
+                invalid_filters.add(arg)
+        elif ":" in arg:
+            tokens = arg.split(":")
+            if len(tokens) != 2:
+                invalid_filters.add(arg)
+        else:
+            invalid_filters.add(arg)
+
+    return invalid_filters
+
+
 def execute(
     autoprotocol,
     api,
@@ -1485,9 +1507,12 @@ def execute(
     schedule_at,
     schedule_delay,
     time_constraints_are_suggestion,
+    exclude,
+    include,
     partition_group_size,
     partition_horizon,
     partitioning_swap_device_id,
+    email,
 ):
     # Clean api end point
     if api.startswith("http://"):
@@ -1508,7 +1533,7 @@ def execute(
             "Error: '--schedule-delay' and '--schedule-at' are mutually exclusive.",
             err=True,
         )
-        return
+        sys.exit(1)
 
     # Get the requested scheduling time
     if schedule_delay is not None:
@@ -1518,12 +1543,27 @@ def execute(
         payload["scheduleAt"] = schedule_at
 
     # Get the autoprotocol
-    autoprotocol_str = autoprotocol.read()
     try:
-        payload["autoprotocol"] = json.loads(autoprotocol_str)
+        autoprotocol_json = json.loads(autoprotocol.read())
+        payload["autoprotocol"] = autoprotocol_json
     except json.decoder.JSONDecodeError as err:
         click.echo(f"Error decoding autoprotocol json: {err}", err=True)
-        return
+        sys.exit(1)
+
+    # validate filters
+    num_instructions = len(autoprotocol_json["instructions"])
+    invalid_exclude = validate_filter(exclude, num_instructions)
+    invalid_include = validate_filter(include, num_instructions)
+    if len(invalid_exclude) + len(invalid_include) > 0:
+        click.echo(
+            f"Error: invalid filters: {','.join(invalid_exclude.union(invalid_include))} (number of instructions: {num_instructions})",
+            err=True,
+        )
+        sys.exit(1)
+
+    # update the payload
+    payload["exclude"] = exclude
+    payload["include"] = include
 
     # device set resolution
     in_use = []
@@ -1534,7 +1574,7 @@ def execute(
             payload["deviceSet"] = device_json
         except json.decoder.JSONDecodeError as err:
             click.echo(f"Error decoding device set json: {err}", err=True)
-            return
+            sys.exit(1)
         in_use.append("--device-set")
 
     if workcell_id:
@@ -1549,7 +1589,7 @@ def execute(
 
     if len(in_use) > 1:
         click.echo(f"Error: {', '.join(in_use)} are mutually exclusive.", err=True)
-        return
+        sys.exit(1)
 
     if len(in_use) == 0:
         payload["workcellIdForDeviceSet"] = "wctest-mcx1"
@@ -1573,9 +1613,10 @@ def execute(
         path_tokens = clean_api.split("/")
         if len(path_tokens) != 3:
             click.echo(
-                f"Invalid api target, expects base-url/facility/workcell.", err=True
+                f"Error: Invalid api target, expects base-url/facility/workcell.",
+                err=True,
             )
-            return
+            sys.exit(1)
 
         clean_api = f"http://{clean_api}"
         path_base = f"http://{path_tokens[0]}"
@@ -1596,16 +1637,22 @@ def execute(
                 ]["url"]
             else:
                 click.echo(
-                    f"Error when get frontend node address: {res_json}", err=True
+                    f"Error when getting frontend node address: {res_json}", err=True
                 )
-                return
+                sys.exit(1)
         except json.decoder.JSONDecodeError:
-            click.echo(f"Error when get frontend node address: {res.text}", err=True)
-            return
+            click.echo(
+                f"Error when getting frontend node address: {res.text}", err=True
+            )
+            sys.exit(1)
+
+    # add sentBy
+    if email is not None:
+        payload["sentBy"] = email.split("@")[0]
 
     # POST to workcell
     test_run_endpoint = f"http://{frontend_node_address}/testRun"
-    click.echo(f"Sending request to http://{frontend_node_address}")
+    click.echo(f"Sending request to {test_run_endpoint}")
     res = requests.post(test_run_endpoint, json=payload)
     try:
         res_json = json.loads(res.text)
@@ -1620,8 +1667,10 @@ def execute(
                     f"Dashboard can be seen at: {clean_api}/dashboard",
                     err=True,
                 )
+                sys.exit(1)
     except json.decoder.JSONDecodeError:
         click.echo(f"Error: {res.text}", err=True)
+        sys.exit(1)
 
 
 def parse_json(json_file):