Skip to content

feat: add task to clean sandbox store#883

Merged
chrisburr merged 7 commits intoDIRACGrid:mainfrom
natthan-pigoux:feat/tasks_clean_sb_store
Apr 14, 2026
Merged

feat: add task to clean sandbox store#883
chrisburr merged 7 commits intoDIRACGrid:mainfrom
natthan-pigoux:feat/tasks_clean_sb_store

Conversation

@chrisburr
Copy link
Copy Markdown
Member

Rebased the work of @natthan-pigoux based on how tasks evolved as a result of his original PR (chrisburr#1).

@aldbr
Copy link
Copy Markdown
Contributor

aldbr commented Apr 8, 2026

Shall we add a unit test?

@chrisburr
Copy link
Copy Markdown
Member Author

diracx.logic.jobs.sandboxes.clean_sandboxes is already tested so here we have a question how we want to handle testing tasks.

I'm not sure what the answer should be for cases like this where tasks is just minimal boiler plate.

from diracx.tasks.plumbing.retry_policies import ExponentialBackoff
from diracx.tasks.plumbing.schedules import CronSchedule

from .depends import NoTransaction
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This looks buggy...

Copy link
Copy Markdown
Contributor

@chaen chaen left a comment

Choose a reason for hiding this comment

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

buggy import

@DIRACGridBot DIRACGridBot marked this pull request as draft April 14, 2026 07:37
@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community bot commented Apr 14, 2026

Documentation build overview

📚 diracx | 🛠️ Build #32252813 | 📁 Comparing bf7b1a2 against latest (09d7149)

  🔍 Preview build  

Show files changed (2 files in total): 📝 2 modified | ➕ 0 added | ➖ 0 deleted
File Status
dev/how-to/use-task-callbacks/index.html 📝 modified
dev/reference/dependency-injection/index.html 📝 modified

@chrisburr
Copy link
Copy Markdown
Member Author

This branch had a broken import in CleanSandboxStoreTask as it imported NoTransaction from a .depends module that didn't exist. This was missed in CI because:

  1. The jobs/ directory was missing __init__.py, so ruff/mypy didn't analyse the file
  2. No test imported the module or loaded the entry point
  3. Entry points are resolved lazily, so the ImportError only surfaces at runtime

I've added several commits on top to fix this and prevent it happening again:

  • Enable ruff INP001 to catch missing __init__.py files (added __init__.py to all test subdirectories and diracx-tasks/src/diracx/tasks/jobs/)
  • Create diracx.tasks.depends as the public re-export module for dependency markers (NoTransaction, CallbackSpawner), so task authors don't need to reach into plumbing internals
  • Fix the broken import in CleanSandboxStoreTask and populate jobs/__init__.py
  • Add a parametrized test that calls .load() on every registered diracx.* entry point
  • Update docs to reference diracx.tasks.depends instead of diracx.tasks.plumbing.depends

Add __init__.py to test subdirectories and diracx-tasks/src/diracx/tasks/jobs/
that were implicit namespace packages. Enable the INP ruff ruleset to prevent
this class of bug in the future.
Re-export NoTransaction from the plumbing layer so task authors
can import from diracx.tasks.depends rather than reaching into
plumbing internals.
Use diracx.tasks.depends instead of the non-existent .depends module.
Populate jobs/__init__.py to re-export CleanSandboxStoreTask as
expected by the entry point definition.
Add a parametrized test that calls .load() on every registered diracx.*
entry point. This catches broken imports that would otherwise only
surface at runtime.
Update documentation examples to import NoTransaction and
CallbackSpawner from diracx.tasks.depends instead of reaching
into diracx.tasks.plumbing.depends. Also add CallbackSpawner
to the public re-export.
@chrisburr chrisburr force-pushed the feat/tasks_clean_sb_store branch from f8067e0 to bf7b1a2 Compare April 14, 2026 09:14
@chrisburr chrisburr marked this pull request as ready for review April 14, 2026 09:24
@chrisburr chrisburr merged commit ab38f04 into DIRACGrid:main Apr 14, 2026
29 checks passed
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