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 Issue #1222: esrally CLI should always return 130 when cancelled #1223

Closed
wants to merge 2 commits into from

Conversation

OBITORASU
Copy link

Description

Previously a sigterm recieved from keyboard interrupt would result in the exit code being 0 and success status being reached, this behaviour was undesired. This PR adds a try and except statement around the possible area of interest and makes the exit status 130 in case of a keyboard interrupt and provides the verbose logging to user.

Fixes Issue #1222

Changes:

  • Error code changed to 130 for keyboard interrupt
  • Abort message is displayed
  • Acknowledgement for keyboard interrupt received is displayed

@dliappis dliappis requested a review from DJRickyB March 29, 2021 06:55
@dliappis dliappis added :Usability Makes Rally easier to use enhancement Improves the status quo labels Mar 29, 2021
@dliappis dliappis added this to the 2.1.0 milestone Mar 29, 2021
@DJRickyB
Copy link
Contributor

Hi @OBITORASU, thank you for your contribution. I am not able to get the CLI to return anything other than SUCCESS (return code 0) using your code. Have you tested it locally? Is there some part of the code that was not pushed?

Thanks

@OBITORASU
Copy link
Author

OBITORASU commented Mar 29, 2021

I have some trouble testing the changes locally, could you please guide me a bit around setting up the tests. Back when I read CONTRIBUTING.md, python setup.py test didn't seem to work so I setup a venv and used python setup.py install. After the setup was done, I ran make test which seemed to succeed, but make it reported cache errors. Running python rally.py race --track=geonames --test-mode inside the venv causes a crash due to JAVA_HOME error. I am using OpenJDK on my Arch Linux system, it is located at /usr/lib/jvm/java-15-openjdk.

EDIT 1: Fixed the JAVA_HOME error. Currently stuck at preparing for race (don't know if that's normal behavior for it to take this long).

EDIT 2: Recreated the bug where cli doesn't return anything but success upon keyboard interrupt, but couldn't get the benchmark running.

@OBITORASU
Copy link
Author

OBITORASU commented Mar 29, 2021

Did some digging regarding this error, couldn't come up with any fixes for this. My current fix doesn't seem to be working and I don't think I would be able to give much time to this error this week (in the middle of some exams), so I am closing my PR in favor of better PRs. Thank you for all the help and support.

@OBITORASU OBITORASU closed this Mar 29, 2021
@dliappis
Copy link
Contributor

@OBITORASU thank you for giving this a go and best of luck with your exams.

@danielmitterdorfer danielmitterdorfer removed this from the 2.1.0 milestone Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :Usability Makes Rally easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants