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

project: fix keyboard interruption for scripts #994

Merged
merged 1 commit into from
May 19, 2020

Conversation

blankoworld
Copy link
Contributor

We often run poetry run server. And we stop it with Ctrl+C. This
generally gives a keyboard interruption signal that stops the processus.

But this is broken: the processus is already runned!

This commit fixes the issue by sending again the interruption signal to
the runned script.

Co-Authored-by: Olivier DOSSMANN [email protected]

Why are you opening this PR?

The poetry run server often let invenio server and celery worker running (after Ctrl+C keyboard interruption). When we launch it again: it returns an error about "Port already addressed".

This PR changes the behaviour by sending an interruption signal to launched script when using a keyboard interruption command.

How to test?

  • Launch poetry run server
  • Wait few seconds (for server to be launched)
  • Interrupt the server with [Ctrl] + [C] command
  • ps aux|grep invenio should not return any launched process
  • ps aux |grep celery should do the same (except if you have other invenio instances launched)

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@blankoworld blankoworld marked this pull request as ready for review May 15, 2020 12:36
@blankoworld blankoworld requested review from BadrAly and rerowep May 15, 2020 12:36
Copy link

@BadrAly BadrAly left a comment

Choose a reason for hiding this comment

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

tested locally and it works nicely

@iGormilhit iGormilhit self-requested a review May 18, 2020 05:12
Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

In the commit message, change "processus" by "process". And "But this is broken: the processus is already runned!" by "But this is broken: the process are still running!", if I got it right.

@blankoworld blankoworld force-pushed the doo-fix-keyboardinterrupt branch 2 times, most recently from 179eafe to 181c4dd Compare May 18, 2020 12:29
We often run `poetry run server`. And we stop it with `Ctrl+C`. This
generally gives a keyboard interruption signal that stops the process.

But this is broken: the process are still running!

This commit fixes the issue by sending again the interruption signal to
the runned script.

Co-Authored-by: Olivier DOSSMANN <[email protected]>
@blankoworld blankoworld force-pushed the doo-fix-keyboardinterrupt branch from 181c4dd to aacbe9a Compare May 18, 2020 13:10
@blankoworld blankoworld requested review from iGormilhit and rerowep May 18, 2020 13:47
@blankoworld blankoworld merged commit f28d021 into rero:dev May 19, 2020
@blankoworld blankoworld deleted the doo-fix-keyboardinterrupt branch May 19, 2020 07:00
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.

4 participants