Skip to content

Conversation

@Kami
Copy link
Member

@Kami Kami commented Apr 18, 2020

This pull request addresses one of my biggest minor annoyances with StackStorm CLI.

A lot of times when I'm debugging / troubleshooting things, I need to view multiple resources at once to see what is going on.

This means that currently I need to run more than one command to achieve that.

For example:

st2 execution get id1
st2 execution get id2
st2 execution get id3

This slows things down and means more typing (yes, I can use a bash alias or function, but that's not ideal and it's not available out of the box).

With this change, user can now view more than one resource using the get command.

The same pattern is already available with st2 execution cancel command (I added it a while ago).

For example:

st2 execution get id1 id2 id3

Example output:

st2 action get packs.unload packs.show
+---------------+--------------------------------------+
| Property      | Value                                |
+---------------+--------------------------------------+
| id            | 5e9ae3535550f016857e19f9             |
| uid           | action:packs:unload                  |
| ref           | packs.unload                         |
| pack          | packs                                |
| name          | unload                               |
| description   | Unregisters all content from a pack. |
| enabled       | True                                 |
| entry_point   | pack_mgmt/unload.py                  |
| runner_type   | python-script                        |
| parameters    | {                                    |
|               |     "packs": {                       |
|               |         "items": {                   |
|               |             "type": "string"         |
|               |         },                           |
|               |         "required": true,            |
|               |         "type": "array"              |
|               |     }                                |
|               | }                                    |
| metadata_file | actions/unload.yaml                  |
| notify        |                                      |
| output_schema |                                      |
| tags          |                                      |
+---------------+--------------------------------------+
+---------------+--------------------------------------------------------------+
| Property      | Value                                                        |
+---------------+--------------------------------------------------------------+
| id            | 5e9ae3535550f016857e19f7                                     |
| uid           | action:packs:show                                            |
| ref           | packs.show                                                   |
| pack          | packs                                                        |
| name          | show                                                         |
| description   | Get detailed information about pack from the remote          |
|               | StackStorm exchange index.                                   |
| enabled       | True                                                         |
| entry_point   | pack_mgmt/show_remote.py                                     |
| runner_type   | python-script                                                |
| parameters    | {                                                            |
|               |     "pack": {                                                |
|               |         "required": true,                                    |
|               |         "type": "string",                                    |
|               |         "description": "Name of pack to lookup"              |
|               |     }                                                        |
|               | }                                                            |
| metadata_file | actions/show.yaml                                            |
| notify        |                                                              |
| output_schema |                                                              |
| tags          |                                                              |
+---------------+--------------------------------------------------------------+

Open Questions / To Decide

Currently when a single source id is provided, the command behaves exactly in the same manner as it did before - it's fully backward compatible.

When retrieving multiple resources, if one of the provided resources is not found, we simply print that, but we don't immediately exit with non-zero status code and print other resources which were found.

As far as using the existing command name goes - I think that's better than adding yet another command (e.g. st2 <resource> get-more or similar) - the whole idea is that it's simple and it works out of the box so any other change which would make it less simple (e.g. new argument or filter to list command, etc.) is a no go.

TODO

  • (Once agreed on the approach) Implement it for other non-resource "get" commands
  • (Once agreed on the approach) Add tests

@Kami Kami added the CLI label Apr 18, 2020
@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Apr 18, 2020
@arm4b arm4b added this to the 3.3.0 milestone Apr 18, 2020
@arm4b arm4b removed this from the 3.3.0 milestone Aug 11, 2020
@stale
Copy link

stale bot commented Nov 9, 2020

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

@stale stale bot added the stale label Nov 9, 2020
@stale stale bot removed the stale label Feb 15, 2021
@Kami
Copy link
Member Author

Kami commented Feb 15, 2021

I've pushed a change which adds this support for all the "resource get" commands.

In addition to that, while working on this change, I cleaned up the code a bit - removed some overriden duplicated methods in sub-classes which are the same as in parent classes, made sure we don't re-hrow ResourceNotFoundError twice in some situations (no need to re-throw it, we can just propagate the original error), fixed inconsistency for "resource not found" errors - most commands would throw ResourceNotFoundError, but a couple would throw / propagate original OperationFailureException.

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Feb 15, 2021
@Kami Kami changed the title [WIP] [RFC] Allow user to retrieve more than one resource using "st2 resource get" CLI command Allow user to retrieve more than one resource using "st2 resource get" CLI command Feb 15, 2021
@Kami Kami added this to the 3.5.0 milestone Feb 15, 2021
@Kami
Copy link
Member Author

Kami commented Feb 16, 2021

I tried to also add some end to end tests, but sadly I don't have many hours to deal with and wait on end to end tests.

So I added some more unit tests which verify this new notation works for all the commands - 986989f.

Manually resolving merge conflicts was too painful due to black changes.
@Kami Kami force-pushed the improve_resource_get_cli_command branch from 369f217 to 62050e1 Compare April 17, 2021 20:23
@Kami
Copy link
Member Author

Kami commented Apr 17, 2021

OK, this is now synced with latest master.

Resolving merge conflicts was too painful due to black changes, so I just manually re-applied my old changes on top of latest master.

@Kami Kami merged commit 5587123 into master Apr 18, 2021
@Kami Kami deleted the improve_resource_get_cli_command branch April 18, 2021 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI size/L PR that changes 100-499 lines. Requires some effort to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants