justinpakzad commented on PR #76106: URL: https://github.com/apache/airflow/pull/76106#issuecomment-6550461309
> **`snowpark_containers.py:300` (`on_kill`):** the task runner calls `on_kill()` on SIGTERM, and `on_kill` drops the service whenever `job_name` is set. A graceful preemption (pod eviction, worker shutdown) therefore destroys the job, and the retry starts fresh, while the new docs say durable mode covers a worker that "crashes or is preempted". Either narrow the docs to hard crashes, or add a `cancel_on_kill` switch like the one `SnowflakeSqlApiOperator` has so users can keep the job running across a preemption. > > **Smaller observations** > > * `docs/operators/snowpark_containers.rst:91-92`: the list of retry outcomes doesn't mention "service no longer exists → submit fresh". "Already succeeded" also still fetches logs and drops the service, so "return immediately" slightly undersells what happens. > > _The `:261` and `on_kill` points were flagged by an adversarial reviewer (codex) and cross-checked against the code._ Thanks for the review. All good callouts. I've addressed the comments. I've added a `cancel_on_kill` parameter as suggested (defaulting to True to maintain the current behavior) and the param is also passed to the triggerer. I re-tested the new changes e2e against a live Snowflake account, simulating SIGTERM kill and ensuring nothing was dropped, and the retry reconnected to the same job when `cancel_on_kill=False` (and some cases where it's True). Docs have also been update to mention we fetch logs and drop the service. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
