linode-cli icon indicating copy to clipboard operation
linode-cli copied to clipboard

Usage on incomplete call unclear

Open nivex opened this issue 3 years ago • 2 comments

First, I ask for help with an incomplete command:

kjotte@mystic:~$ linode-cli networking ip-update
usage: linode-cli [-h] [--rdns rdns] address
linode-cli: error: the following arguments are required: address

First, the example usage doesn't include all the arguments, eg: networking ip-update. Yes, I just provided that, but the readback should probably be consistent.

OK, I'll provide the only mandatory argument, since --rdns rdns is in brackets, indicating optional:

kjotte@mystic:~$ linode-cli networking ip-update 2600:3c03:e001:601::d01
Request failed: 400
┌errors─┬──────────────────┐
│ field │ reason           │
├───────┼──────────────────┤
│ rdns  │ rdns is required │
└───────┴──────────────────┘

Makes sense to me, a seasoned network person, that it's required, but this may not be obvious to a newer user. The [ ] should probably be removed from the usage hint, and --rdns dns be added to "the following arguments are required".

nivex avatar Nov 19 '21 20:11 nivex

Thank you for the report!

It looks like the help output is what you expect it to be:

$ linode-cli networking ip-update --help
linode-cli networking ip-update [ADDRESS]
IP Address RDNS Update

Arguments:
  --rdns: The reverse DNS assigned to this address.

The only thing is, we're only displaying that right now if --help is given, and it appears that the error output isn't nearly as helpful. This should certainly be improvied.

It's also worth noting that rdns isn't marked as required in the help output; I'll see if I can get that addressed, as that seems to be an error in how this endpoint is documented.

Dorthu avatar Nov 22 '21 12:11 Dorthu

I opened #274 to handle the first issue.

The second one (rdns not being included in the required arguments) is slightly more tricky to address; the CLI supports configurable default values for some arguments (i.e. --region), and the way those are implemented today doesn't work with the behavior of python's ArgParse (which is used for parsing command line options at present). I'll dp some thinking on how to get those two systems to cooperate and circle back to this.

Dorthu avatar Nov 22 '21 12:11 Dorthu

I've put up a PR on linode-api-docs to resolve the documentation inconsistencies with the rdns field, and a PR here to properly format required fields on update operations.

The help output for ip-update should show --rdns as a required field after the two PRs are merged and released.

There is still the issue of setting rdns to null through the CLI, but I'd like to move that conversation to #266 to keep things organized :slightly_smiling_face:

LBGarber avatar Oct 05 '22 14:10 LBGarber

This change has been resolved in 5.24.0 :tada:

lgarber-akamai avatar Oct 18 '22 19:10 lgarber-akamai