adds more signal options #3248
adds more signal options #3248
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | accc93b | Commit Preview URL Branch Preview URL |
Oct 28 2025, 12:00 AM |
… runnable load job
…not complete package if pool drained but jobs left, adds detailed tests for metrics
rudolfix
force-pushed
the
feat/more-signal-options
branch
from
e69bba7 to
1bfaf20
Compare
rudolfix
force-pushed
the
feat/more-signal-options
branch
from
1bfaf20 to
fb44a6a
Compare
There was a problem hiding this comment.
Cool PR!
I have one major question around how the Semaphore works, I am missing something there.
Apart from that, the docs are quite cryptic imho and the config setting intercept_signals is not self-explanatory at all. Maybe handle_signals_gracefully or something like that would be nicer.
| - `normalize` step: raises `SignalReceivedException` at certain checkpoints, typically immediately | ||
| - `load` step: on a first received signal, it attempts to drain the job pool by not accepting new load jobs but waiting for executing jobs to complete. | ||
| On a second signal, default handler is called resulting in KeyboardInterrupt or immediate process termination (SIGTERM) | ||
| - `extract` step does not intercept signals and uses default handlers. However it will raise `SignalReceivedException` is `dlt` custom signalling is activated. |
There was a problem hiding this comment.
To me it is not cleaer what "dlt custom signalling" means
| ::: | ||
|
|
||
| #### Write custom signal handler | ||
| You can install your own signal handler. We are working on making the `signals.py` pluggable. Currently you can just disable signal interception globally |
There was a problem hiding this comment.
Here I would also explain better what "signal interception" means. I know what "intercept_signals" does when reading the code, but if I were only to read the docs, I would not know.
| load_info = load() | ||
| ``` | ||
|
|
||
| ### Allow for graceful shutdown |
There was a problem hiding this comment.
Maybe run the grammar checker on this whole block.
There was a problem hiding this comment.
yeah you are right I'll do that :/
| self._job_metrics: Dict[str, LoadJobMetrics] = {} | ||
| # job completed signalling event (start blocking) | ||
| self._done_event = BoundedSemaphore() | ||
| self._done_event.acquire() |
There was a problem hiding this comment.
This I don't understand. You acquire this once on the main thread but every individual job releases it on the job thread? How does this work? I think I am missing some part of this, I would expect an exception if there are more than 2 jobs.
There was a problem hiding this comment.
that's actually trivial: semaphore is created in release state so first acquire will just get through. I want first loop in load to sleep. if I do not acquire semaphore right away - it skips. btw. typically you can set if semaphore is released or not (initial value) in constructor. but not in Python apparently
rudolfix
force-pushed
the
feat/more-signal-options
branch
from
5f7b96b to
accc93b
Compare
2 participants
Description
More signal handling options for typical production cases. See commit log and docs update