Skip to content

feat: allow use of server.name as server_id argument #11

Merged
nats merged 26 commits intobinarylane:mainfrom
nats:feat/x-cli-lookup
Apr 20, 2023
Merged

feat: allow use of server.name as server_id argument #11
nats merged 26 commits intobinarylane:mainfrom
nats:feat/x-cli-lookup

Conversation

@nats
Copy link
Copy Markdown
Contributor

@nats nats commented Feb 20, 2023 •
edited
Loading

This PR modifies parser for operations that accept an integer server_id path parameter to accept a string command-line argument. For example, if account has a server named foo-bar.bnr.la here are some example commands that are now valid:

bl server get foo-bar.bnr.la
bl server delete foo-bar.bnr.la
bl server console foo-bar.bnr.la
bl server action reboot foo-bar.bnr.la

During parsing, if the command-line argument provided for server_id cannot be converted to integer (i.e. int(argument) raises ValueError) then the argument is assumed to be the name of a server. The server list operation is used to fetch all servers, and the result filtered to find the integer ID of the server with name == argument.

If there is no such server, the parser displays an error:

error: SERVER_ID: cound not find 'foo-bar.bnr.la'

nats added 20 commits February 20, 2023 10:35
@nats nats changed the title feat: use spec "x-cli-lookup" to convert server name to ID feat: allow use of server.name as server_id argument Apr 1, 2023
@nats nats marked this pull request as ready for review April 1, 2023 02:54
@nats nats requested a review from david-pershouse April 1, 2023 02:54
@nats
Copy link
Copy Markdown
Contributor Author

nats commented Apr 1, 2023

You currently just have to 'know' this is supported, since the displayed help for server_id is pulled from the OpenAPI spec:

$ grep '"server_id"' -A 4 src/binarylane/console/commands/api/* | grep -oP "description=.*" | sort | uniq -c
    1 description="""The ID of the server for which actions should be listed.""",
      1 description="""The ID of the server for which advanced features should be listed.""",
      1 description="""The ID of the server for which backups should be listed.""",
      1 description="""The ID of the server for which console URLs will be fetched.""",
      1 description="""The ID of the server for which firewall rules should be listed.""",
      1 description="""The ID of the server for which kernels should be listed.""",
      1 description="""The ID of the server for which snapshots should be listed.""",
      1 description="""The ID of the server for which software should be fetched.""",
      1 description="""The ID of the server for which the action should be fetched.""",
      1 description="""The ID of the server for which the backup is to be uploaded.""",
      1 description="""The ID of the server for which threshold alerts should be fetched.""",
      1 description="""The ID of the server for which userdata should be fetched.""",
     41 description="""The ID of the server on which the action should be performed.""",
      1 description="""The ID of the server to be cancelled.""",
      1 description="""The ID of the server to fetch.""",
      6 description="""The target server id.""",

@david-pershouse not sure how best to address that, any thoughts?

@david-pershouse
Copy link
Copy Markdown
Contributor

david-pershouse commented Apr 4, 2023

A simple if not a little hacky way would be to append to the help text based on _lookup.

if self._lookup
   self.description = self.description.replace("ID", "ID or {entity['ref']}")

in the presence of lookup we could also change the param name SERVER_ID -> SERVER (eg strip any trailing _ID if a ref is available)

david-pershouse
david-pershouse requested changes Apr 4, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@david-pershouse david-pershouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _context passed to create_client in the lookup operations doesn't have the correct config. If I pass --context dev on the command line, the lookup runner ignores it and hits api.binarylane.com to resolve the name, then actual requested command executes against the dev context as expected.

@nats nats requested a review from david-pershouse April 17, 2023 01:00
david-pershouse
david-pershouse reviewed Apr 17, 2023
View reviewed changes
david-pershouse
david-pershouse reviewed Apr 17, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@david-pershouse david-pershouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean solution, confirms it fixes it here

@nats nats requested a review from david-pershouse April 20, 2023 03:00
@nats nats merged commit 1b9ef5b into binarylane:main Apr 20, 2023
@nats nats deleted the feat/x-cli-lookup branch April 20, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@david-pershouse david-pershouse david-pershouse approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nats @david-pershouse