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

stall-analyser: gracefully handle empty input #2305

Conversation

bhalevy
Copy link
Member

@bhalevy bhalevy commented Jun 27, 2024

Currently, the tools prints an obscure error
if the input is empty:

Traceback (most recent call last):
  File "/home/bhalevy/dev/scylla_s3_reloc_server/./seastar//scripts/stall-analyser.py", line 363, in <module>
    print_stats(tally, tmin)
                       ^^^^
NameError: name 'tmin' is not defined. Did you mean: 'min'?

Instead, print a meaningful error and direct the user to run stall-analyser.py --help for usage instructions.

Ref https://github.com/scylladb/scylla_s3_reloc_server/pull/103

if t >= tmin:
graph.process_trace(trace, t)

try:
if not graph:
print("No input data found. Please run `stall-analyser.py --help` for usage instruction")
exit(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd suggest use sys.exit() instead. see https://docs.python.org/3/library/constants.html#constants-added-by-the-site-module . i am quoting the remarks in the document.

They are useful for the interactive interpreter shell and should not be used in programs.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Currently, the tools prints an obscure error
if the input is empty:
```
Traceback (most recent call last):
  File "/home/bhalevy/dev/scylla_s3_reloc_server/./seastar//scripts/stall-analyser.py", line 363, in <module>
    print_stats(tally, tmin)
                       ^^^^
NameError: name 'tmin' is not defined. Did you mean: 'min'?
```

Instead, print a meaningful error and direct the user
to run stall-analyser.py --help for usage instructions.

Ref scylladb/scylla_s3_reloc_server#103

Signed-off-by: Benny Halevy <[email protected]>
@bhalevy bhalevy force-pushed the stall-annalyser-handle-empty-input-gracefully branch from 12480e9 to f0b62cb Compare June 27, 2024 14:11
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

lgtm

@bhalevy
Copy link
Member Author

bhalevy commented Jun 28, 2024

@scylladb/seastar-maint please merge

@xemul xemul closed this in f09e7c4 Jun 28, 2024
ptrsmrn pushed a commit to ptrsmrn/seastar that referenced this pull request Jul 17, 2024
Currently, the tools prints an obscure error
if the input is empty:
```
Traceback (most recent call last):
  File "/home/bhalevy/dev/scylla_s3_reloc_server/./seastar//scripts/stall-analyser.py", line 363, in <module>
    print_stats(tally, tmin)
                       ^^^^
NameError: name 'tmin' is not defined. Did you mean: 'min'?
```

Instead, print a meaningful error and direct the user
to run stall-analyser.py --help for usage instructions.

Ref scylladb/scylla_s3_reloc_server#103

Signed-off-by: Benny Halevy <[email protected]>

Closes scylladb#2305
lovio pushed a commit to lovio/seastar that referenced this pull request Aug 30, 2024
Currently, the tools prints an obscure error
if the input is empty:
```
Traceback (most recent call last):
  File "/home/bhalevy/dev/scylla_s3_reloc_server/./seastar//scripts/stall-analyser.py", line 363, in <module>
    print_stats(tally, tmin)
                       ^^^^
NameError: name 'tmin' is not defined. Did you mean: 'min'?
```

Instead, print a meaningful error and direct the user
to run stall-analyser.py --help for usage instructions.

Ref https://github.com/scylladb/scylla_s3_reloc_server/pull/103

Signed-off-by: Benny Halevy <[email protected]>

Closes scylladb#2305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants