Skip to content

Update dig.py error handling#4732

Merged
blag merged 12 commits intoStackStorm:masterfrom
punkrokk:master
Jul 23, 2019
Merged

Update dig.py error handling#4732
blag merged 12 commits intoStackStorm:masterfrom
punkrokk:master

Conversation

@punkrokk
Copy link
Member

@punkrokk punkrokk commented Jun 30, 2019 •
edited
Loading

I have added an exception to the linux.dig action script to handle the situation where the dig command is not installed, which, it seems on CentOS/RHEL gets the user this error:

{
  "stdout": "",
  "result": "None",
  "stderr": "Traceback (most recent call last):
  File \"/opt/stackstorm/st2/lib/python2.7/site-packages/python_runner/python_action_wrapper.py\", line 334, in <module>
    obj.run()
  File \"/opt/stackstorm/st2/lib/python2.7/site-packages/python_runner/python_action_wrapper.py\", line 193, in run
    output = action.run(**self._parameters)
  File \"/opt/stackstorm/packs/linux/actions/dig.py\", line 30, in run
    stdout=subprocess.PIPE)
  File \"/usr/lib64/python2.7/subprocess.py\", line 711, in __init__
    errread, errwrite)
  File \"/usr/lib64/python2.7/subprocess.py\", line 1327, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
",
  "exit_code": 1
}

This doesn't seem to be an issue on Ubuntu. It appears the dnsutils is installed. At least on the vagrant st2 dev environment. As you can see there is now a better error message if we get an OSError exception, suggesting that dig is either not on the PATH or installed.

I have also update the linux pack readme to include the dig action.

Since this is a core pack and installed by default, I opened the below PR to install bind-utils by default in EL7 distros.
StackStorm/st2-packages#619

blag
blag suggested changes Jul 2, 2019
View reviewed changes
@punkrokk
Copy link
Member Author

punkrokk commented Jul 12, 2019

@blag updated based on your requests.

@arm4b arm4b added this to the 3.2.0 milestone Jul 15, 2019
arm4b
arm4b reviewed Jul 15, 2019
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Will wait for @blag for his review,

but a small nitpick from me: I think this improvement will also need a CHANGELOG record since it's part of st2 core.

@arm4b arm4b requested a review from blag July 19, 2019 12:15
blag
blag suggested changes Jul 19, 2019
View reviewed changes
Copy link
Contributor

@blag blag left a comment

Choose a reason for hiding this comment

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

One small wording change, and add a changelog please.

Other than that, this looks good to go! 👍

@blag blag self-requested a review July 23, 2019 19:54
@blag blag merged commit 4f78ed9 into StackStorm:master Jul 23, 2019
@blag
Copy link
Contributor

blag commented Jul 23, 2019

Thanks for the contribution!

@arm4b arm4b mentioned this pull request Apr 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@arm4b arm4b arm4b left review comments

@blag blag blag approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement external dependency python3 runners

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@punkrokk @blag @arm4b