Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "st2 action-alias execute" CLI command #5138

Merged
merged 7 commits into from
Feb 11, 2021
Merged

Conversation

Kami
Copy link
Member

@Kami Kami commented Feb 7, 2021

I tried to use StackStorm after quite a long time and in the first 5 minutes I encountered two exceptions / bugs (like in the good old days - guess I'm always the "lucky" one :D).

There are two bugs in st2 action-alias execute command so it doesn't work.

  1. If user query param is not specified, it will fail with the error below due to API returning an error due to type mismatch (out of the box behavior with st2-docker).
# -------- begin 140132975715104 request ----------
curl -X POST -H  'User-Agent: python-requests/2.23.0' -H  'Accept-Encoding: gzip, deflate' -H  'Accept: */*' -H  'Connection: keep-alive' -H  'X-Auth-Token: XXXX' -H  'content-type: application/json' -H  'Content-Length: 61' --data-binary '{"command": "foo=bar", "user": null, "source_channel": "cli"}' http://st2api:9101/aliasexecution/match_and_execute
# -------- begin 140132975715104 response ----------
{
    "faultstring": "None is not of type 'string'"
}
  1. Result for match_and_execute API operation is not correctly de-serialized in the client so client returns empty object and the code throws an exception. API returns a container object with a list so we need to handle that correctly.
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2client/shell.py", line 408, in run
    func(args)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2client/commands/action_alias.py", line 127, in run_and_print
    print("Matching Action-alias: '%s'" % execution.actionalias['ref'])
AttributeError: 'ActionAliasExecution' object has no attribute 'actionalias'

This PR fixes both issues.

TODO

  • Add tests (no idea when I'll get a chance to do it, but for sure we need tests because I assume there are not otherwise they would have caught this bug)

There were two bugs:

1. If user it's not specified it will return an error
2. It didn't correctly de-serialize the result so it failed
@pull-request-size pull-request-size bot added the size/XS PR that changes 0-9 lines. Quick fix/merge. label Feb 7, 2021
@pull-request-size pull-request-size bot added size/S PR that changes 10-29 lines. Very easy to review. and removed size/XS PR that changes 0-9 lines. Quick fix/merge. labels Feb 8, 2021
@Kami
Copy link
Member Author

Kami commented Feb 8, 2021

Adding direct st2client tests will be a bit of a pain so to begin with, I added in-direct test for the API endpoint itself (5df4e8e - similar to the ones in #5141) which verify that API endpoint indeed returns a list with specific key present.

Kami added 4 commits February 8, 2021 16:20
captured stdout and stderr to display captured stdout + stderr on test
failure.

This should make troubleshooting those tests much easier and faster.
@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Feb 8, 2021
@Kami
Copy link
Member Author

Kami commented Feb 8, 2021

OK, I couldn't stand not having any tests for such a basic functionality so I also added direct tests in 19e2206 (still not ideal since response is mocked, but that's the pattern we use for CLI tests :/).

I also pushed a small related improvements which such make troubleshooting CLI tests easier - 75a114c.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Wow. I didn't know about st2 action-alias execute. Awesome.
Fixes and tests lgtm

@Kami
Copy link
Member Author

Kami commented Feb 8, 2021

I'm also working on related functionality - execute-and-format which will run the execution which matches an alias and also format and print the result - this will make testing ChatOps easier and faster since you will be able to do it quickly on command line and no need to do it via chat.

@Kami Kami added this to the 3.4.0 milestone Feb 11, 2021
@Kami Kami merged commit 1ad0063 into master Feb 11, 2021
@Kami Kami deleted the fix_action_alias_cmd branch February 11, 2021 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:st2client 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.

3 participants