fix: job deletion propagation policy

[gilbsgilbs]

The previous way of setting the propagation policy had no effect. As a result, child resources such as pods (among others) would not be properly destroyed.

see kubernetes-client/python#234

Summary by CodeRabbit

• Bug Fixes
  • Simplified job deletion handling to improve reliability and ensure proper cleanup of resources in Kubernetes environments while maintaining error handling behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified the safe_delete_job method to remove explicit pod deletion logic and instead rely on Kubernetes' foreground deletion propagation when deleting the job, while maintaining the deletion request with propagation policy and existing error handling.

Changes

Cohort / File(s)	Summary
Kubernetes job deletion logic snakemake_executor_plugin_kubernetes/__init__.py	Removed manual pod enumeration and deletion block; adjusted safe_delete_job to use foreground deletion propagation policy through V1DeleteOptions body parameter instead of explicit method parameter; preserved 404 error handling for ApiException

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring attention:
  • Verify foreground deletion propagation policy is correctly honored by passing it in the delete options body rather than as a parameter
  • Confirm that relying on Kubernetes' cascade deletion (via foreground propagation) properly cleans up associated pods without orphaning them
  • Validate that the error handling for 404 conditions still functions correctly with the new deletion request structure

Possibly related PRs

• fix: delete k8s jobs after failure (important in setups where the system attaches sidecar pods, which can otherwise remain after failure) #49: Conflicting approach to pod cleanup in safe_delete_job—this PR removes manual pod deletion, while PR #49 explicitly lists and deletes pods before job deletion.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: job deletion propagation policy' directly describes the main change: fixing how deletion propagation policy is set for job deletion in the Kubernetes executor plugin.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5f720e and 407d342.

📒 Files selected for processing (1)

• snakemake_executor_plugin_kubernetes/__init__.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

• snakemake_executor_plugin_kubernetes/__init__.py

🧠 Learnings (1)

📚 Learning: 2024-10-08T17:41:54.542Z

Learnt from: johanneskoester
Repo: snakemake/snakemake-executor-plugin-kubernetes PR: 22
File: snakemake_executor_plugin_kubernetes/__init__.py:32-39
Timestamp: 2024-10-08T17:41:54.542Z
Learning: Do not suggest type annotation of the `self` argument of methods in the Snakemake executor plugin for Kubernetes.

Applied to files:

• snakemake_executor_plugin_kubernetes/__init__.py

🔇 Additional comments (1)

snakemake_executor_plugin_kubernetes/__init__.py (1)

592-600: Foreground V1DeleteOptions usage correctly fixes propagation policy

Setting propagation_policy="Foreground" on V1DeleteOptions and passing it as the body to delete_namespaced_job is the correct way to get foreground deletion with the Python Kubernetes client. This allows Job deletion to clean up its pods without manual pod-deletion logic, and the surrounding error handling (ignore_not_found) is preserved. No remaining call sites using ineffective propagation_policy kwarg patterns were found in the codebase.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}