Fix SIGINT handler for StopOnExitJobServer#34748
Conversation
lostluck
left a comment
There was a problem hiding this comment.
That definitely explains the exception message. Good find!
|
Run Python_Dataframes PreCommit 3.11 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #34748 +/- ##
============================================
+ Coverage 56.48% 56.50% +0.02%
Complexity 3289 3289
============================================
Files 1178 1180 +2
Lines 180978 181262 +284
Branches 3399 3399
============================================
+ Hits 102220 102419 +199
- Misses 75499 75584 +85
Partials 3259 3259
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Does this help with #33623 in combination with the Singleton experiment? |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @claudevdm for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Run Python_Runners PreCommit 3.9 |
Yes, there are multiple changes recently to address #33623.
'PrismJobServer' is wrapped around StopOnExitJobServer', which is then made into a singleton. After this PR, when the event of SIGINT (eg. pressing the stop button in Colab or Ctrl + c in terminal) is triggered, the prism server will stop gracefully rather than being defunct'ed. The singleton of 'PrismJobServer' on the other hand is still valid (though there is no prism subprocess running atm), and we will be able to start the subprocess again if we run another pipeline on the same Colab. The current behavior is the same for both Colab and Terminal. A future improvement is that we can keep the prism server alive on the event of SIGINT in an interactive environment and, as @lostluck suggested in another thread, enforce an idle timeout. |
|
Fantastic! |
The previously registered SIGINT handler had an incorrect signature according to the signal module documentation:
https://docs.python.org/3/library/signal.html#signal.signal
This PR updates the handler to the correct signature. It now gracefully stops the server and ensures the original handler is called for proper application exit. This is important especially in a Colab environment, because the kernel will send out SIGINT when the stop button of a cell is pressed.