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 linux.traceroute stdout formatting #3175

Merged
merged 2 commits into from
Feb 1, 2017
Merged

Conversation

arm4b
Copy link
Member

@arm4b arm4b commented Feb 1, 2017

Reported in Slack community by @livelace

Original traceroute.sh which is part of linux.traceroute action doesn't preserve line separators, what's confusing for users, thinking that StackStorm itself eat some line separators.

@livelace By design StackStorm shows what he get from the script via stdout, stderr and exit code.

This small change should fix linux.traceroute action.

Before

./traceroute.sh google.com

traceroute to google.com (216.58.209.174), 30 hops max, 60 byte packets ...ae-3-3201.bar1.Budapest1.Level3.net (4.69.201.158) 24.733 ms 24.775 ms 24.971 ms 8 72.14.243.192 (72.14.243.192) 24.934 ms 24.892 ms 24.850 ms 9 209.85.243.119 (209.85.243.119) 45.224 ms 25.112 ms 25.077 ms 10 66.249.95.61 (66.249.95.61) 25.038 ms 25.150 ms 25.224 ms 11 bud02s21-in-f174.1e100.net (216.58.209.174) 25.183 ms 25.142 ms 24.755 ms

After

$ ./traceroute.sh google.com
traceroute to google.com (216.58.209.174), 30 hops max, 60 byte packets
...
 5  * * *
 6  ae-3-3201.bar1.Budapest1.Level3.net (4.69.201.158)  25.311 ms  24.348 ms  24.289 ms
 7  ae-3-3201.bar1.Budapest1.Level3.net (4.69.201.158)  24.247 ms  24.289 ms  24.229 ms
 8  72.14.243.192 (72.14.243.192)  24.426 ms  24.139 ms  24.392 ms
 9  209.85.243.119 (209.85.243.119)  24.501 ms  25.213 ms  24.424 ms
10  66.249.95.61 (66.249.95.61)  24.581 ms  24.549 ms  24.754 ms
11  bud02s21-in-f174.1e100.net (216.58.209.174)  24.700 ms  24.708 ms  24.488 ms

@codecov-io
Copy link

codecov-io commented Feb 1, 2017

Codecov Report

Merging #3175 into master will increase coverage by -0.02%.

@@            Coverage Diff             @@
##           master    #3175      +/-   ##
==========================================
- Coverage    77.5%   77.48%   -0.02%     
==========================================
  Files         431      431              
  Lines       22264    22264              
==========================================
- Hits        17255    17251       -4     
- Misses       5009     5013       +4
Impacted Files Coverage Δ
...on/st2common/transport/connection_retry_wrapper.py 75% <ø> (-13.46%)
st2actions/st2actions/scheduler.py 83.67% <ø> (-6.12%)
st2common/st2common/bootstrap/sensorsregistrar.py 78.64% <ø> (-3.88%)
st2common/st2common/services/triggerwatcher.py 85.92% <ø> (-2.82%)
st2api/st2api/controllers/v1/actionexecutions.py 87.39% <ø> (-2.17%)
st2actions/st2actions/policies/concurrency.py 97.87% <ø> (-2.13%)
st2common/st2common/rbac/decorators.py 84.38% <ø> (-1.56%)
st2common/st2common/services/triggers.py 91.75% <ø> (-1.55%)
st2api/st2api/controllers/v1/timers.py 76.92% <ø> (-1.28%)
st2actions/st2actions/notifier/notifier.py 89.17% <ø> (-1.27%)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4c1e8b...f55138b. Read the comment docs.

@arm4b arm4b mentioned this pull request Feb 1, 2017
@arm4b arm4b requested a review from bigmstone February 1, 2017 13:49
Copy link
Contributor

@bigmstone bigmstone left a comment

Choose a reason for hiding this comment

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

👍

@arm4b arm4b merged commit 34f3237 into master Feb 1, 2017
@arm4b arm4b deleted the fix/linux.traceroute-stdout branch February 1, 2017 22:38
@Kami
Copy link
Member

Kami commented Feb 6, 2017

Thanks, good catch 👍

Would it also be possible to add some kind of test for it? :)

@cognifloyd cognifloyd removed the RFR label Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants