Skip to content

Commit

Permalink
new: Improve handling for list structures (#456)
Browse files Browse the repository at this point in the history
## 📝 Description

This change improves the handling for nested list structures (e.g.
Instance Interfaces) by implicitly detecting when the next list entry
should be populated. This means that users do not have to explicitly
specify empty values for null fields.

For example, see this command:

```bash
linode-cli linodes config-update 1234 5678 \
--interfaces.purpose public \
--interfaces.purpose vlan --interfaces.label my-vlan 
```

A user would intuitively expect this command to set the first interface
to public and the second interface to a VLAN with the label `my-vlan`,
however executing this command would generate this request body:

```json
{
   "interfaces":[
      {
         "label":"cool",
         "purpose":"public"
      }
   ]
}
```

After this change, the request body is now generated as expected:

```json
{
   "interfaces":[
      {
         "label":null,
         "ipam_address":null,
         "purpose":"public"
      },
      {
         "label":"cool",
         "purpose":"vlan"
      }
   ]
}
```


## ✔️ How to Test

```
make test
```
  • Loading branch information
lgarber-akamai authored May 19, 2023
1 parent 7bdf730 commit 80e4c6d
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 1 deletion.
10 changes: 10 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,16 @@ you can execute the following::

linode-cli linodes create --region us-east --type g6-nanode-1 --tags tag1 --tags tag2

Lists consisting of nested structures can also be expressed through the command line.
For example, to create a Linode with a public interface on ``eth0`` and a VLAN interface
on ``eth1`` you can execute the following::

linode-cli linodes create \
--region us-east --type g6-nanode-1 --image linode/ubuntu22.04 \
--root_pass "myr00tp4ss123" \
--interfaces.purpose public \
--interfaces.purpose vlan --interfaces.label my-vlan

Specifying Nested Arguments
"""""""""""""""""""""""""""

Expand Down
54 changes: 53 additions & 1 deletion linodecli/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,58 @@ def __call__(self, parser, namespace, values, option_string=None):
raise argparse.ArgumentTypeError("Expected a string")


class ListArgumentAction(argparse.Action):
"""
This action is intended to be used only with list arguments.
Its purpose is to aggregate adjacent object fields and produce consistent
lists in the output namespace.
"""

def __call__(self, parser, namespace, values, option_string=None):
if getattr(namespace, self.dest) is None:
setattr(namespace, self.dest, [])

dest_list = getattr(namespace, self.dest)
dest_length = len(dest_list)
dest_parent = self.dest.split(".")[:-1]

# If this isn't a nested structure,
# append and return early
if len(dest_parent) < 1:
dest_list.append(values)
return

# A list of adjacent fields
adjacent_keys = [
k
for k in vars(namespace).keys()
if k.split(".")[:-1] == dest_parent
]

# Let's populate adjacent fields ahead of time
for k in adjacent_keys:
if getattr(namespace, k) is None:
setattr(namespace, k, [])

adjacent_items = {k: getattr(namespace, k) for k in adjacent_keys}

# Find the deepest field so we can know if
# we're starting a new object.
deepest_length = max(len(x) for x in adjacent_items.values())

# If we're creating a new list object, append
# None to every non-populated field.
if dest_length >= deepest_length:
for k, item in adjacent_items.items():
if k == self.dest:
continue

if len(item) < dest_length:
item.append(None)

dest_list.append(values)


TYPES = {
"string": str,
"integer": int,
Expand Down Expand Up @@ -244,7 +296,7 @@ def parse_args(
parser.add_argument(
"--" + arg.path,
metavar=arg.name,
action="append",
action=ListArgumentAction,
type=TYPES[arg.arg_type],
)
list_items.append((arg.path, arg.list_item))
Expand Down
80 changes: 80 additions & 0 deletions tests/unit/test_operation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import argparse

from linodecli import operation


class TestOperation:
def test_list_arg_action_basic(self):
"""
Tests a basic list argument condition.
"""

parser = argparse.ArgumentParser(
prog=f"foo",
)

for arg_name in ["foo", "bar", "aaa"]:
parser.add_argument(
f"--foo.{arg_name}",
metavar=arg_name,
action=operation.ListArgumentAction,
type=str,
)

result = parser.parse_args(
[
"--foo.foo",
"cool",
"--foo.bar",
"wow",
"--foo.aaa",
"computer",
"--foo.foo",
"test",
"--foo.bar",
"wow",
"--foo.aaa",
"akamai",
]
)
assert getattr(result, "foo.foo") == ["cool", "test"]
assert getattr(result, "foo.bar") == ["wow", "wow"]
assert getattr(result, "foo.aaa") == ["computer", "akamai"]

def test_list_arg_action_missing_attr(self):
"""
Tests that a missing attribute for the first element will be
implicitly populated.
"""

parser = argparse.ArgumentParser(
prog=f"foo",
)

for arg_name in ["foo", "bar", "aaa"]:
parser.add_argument(
f"--foo.{arg_name}",
metavar=arg_name,
action=operation.ListArgumentAction,
type=str,
)

result = parser.parse_args(
[
"--foo.foo",
"cool",
"--foo.aaa",
"computer",
"--foo.foo",
"test",
"--foo.bar",
"wow",
"--foo.foo",
"linode",
"--foo.aaa",
"akamai",
]
)
assert getattr(result, "foo.foo") == ["cool", "test", "linode"]
assert getattr(result, "foo.bar") == [None, "wow"]
assert getattr(result, "foo.aaa") == ["computer", None, "akamai"]

0 comments on commit 80e4c6d

Please sign in to comment.