feat: Check for __all__ presence in __init__.py files

[AcquaDiGiorgio]

See #426

I tried something more sofisticated in python using importlib to check the variables in the module, both instantiated and imported, but there were multiple situations that were extremely difficult to check.

We also might want to set a common way of defining this variable, using either tuples or lists:

• diracx/routers/health/__init__.py: __all__ = ["router"]
• diracx/db/os/__init__.py __all__ = ("JobParametersDB",)

[read-the-docs-community]

Documentation build overview

📚 diracx | 🛠️ Build #31299029 | 📁 Comparing 92abfc9 against latest (9cc792e)

🔍 Preview build

No files changed.

[chrisburr]

I think you need to exclude the auto-generated part of diracx-client from this i.e.

https://github.com/DIRACGrid/diracx/tree/main/diracx-client/src/diracx/client/_generated

I guess this should also apply to the example extension (gubbins):

https://github.com/DIRACGrid/diracx/tree/main/extensions/gubbins

[AcquaDiGiorgio]

The regex only applies for files that start with diracx-, so the extensions directory won't be searched.

Also, with the exclude: _generated in the next line, it already ignores the auto-generated files.

With the current configuration, it finds the following files:

diracx-logic/src/diracx/logic/task_queues/__init__.py
diracx-logic/src/diracx/logic/jobs/__init__.py
diracx-logic/src/diracx/logic/auth/__init__.py
diracx-logic/src/diracx/logic/__init__.py

diracx-db/src/diracx/db/sql/dummy/__init__.py
diracx-db/src/diracx/db/sql/job/__init__.py
diracx-db/src/diracx/db/sql/pilot_agents/__init__.py
diracx-db/src/diracx/db/sql/task_queue/__init__.py
diracx-db/src/diracx/db/sql/job_logging/__init__.py
diracx-db/src/diracx/db/sql/sandbox_metadata/__init__.py
diracx-db/src/diracx/db/sql/auth/__init__.py
diracx-db/tests/pilot_agents/__init__.py

diracx-core/src/diracx/core/__init__.py

diracx-routers/src/diracx/routers/utils/__init__.py
diracx-routers/src/diracx/routers/jobs/__init__.py

[AcquaDiGiorgio]

Or do you mean that the gubbins extension should be checked too?

[aldbr]

Yes I think they should also be checked indeed

[aldbr]

We also might want to set a common way of defining this variable, using either tuples or lists:

I didn't see any standard way of defining __all__ but indeed, it would be better if consistent.
From what I observed, the list is generally preferred (https://stackoverflow.com/questions/66098828/using-list-instead-of-tuple-in-module-all).

[aldbr]

Any specific reason for making a bash script instead of a pixi task as done here:

diracx/.pre-commit-config.yaml

Lines 86 to 102 in 9cc792e

	- repo: local
	hooks:
	- id: generate-pixi-docs
	name: Generate pixi tasks documentation
	entry: pixi run -e default python .github/workflows/generate_pixi_tasks_doc.py
	language: system
	pass_filenames: false
	files: ^pixi\.toml$|^pixi\.lock$ # only run if pixi files change
	- repo: local
	hooks:
	- id: settings-doc-check
	name: Generate settings documentation
	entry: pixi run -e default python scripts/generate_settings_docs.py
	language: system
	pass_filenames: false
	files: ^(diracx-.*/src/diracx/.*/settings\.py|docs/.*\.j2|docs/templates/.*\.jinja|scripts/generate_settings_docs\.py)$

If we want to enforce lists for instance, I have the feeling it would be easier to implement (and also let the possibility of unit testing it if needed).

Any opinion?

[AcquaDiGiorgio]

I used bash for the commodity of grep, but I can change the script to use python with pixi instead, it will definetely be much easier/cleaner to check its type that way.

[chrisburr]

Yet another fake review for me to be able to test something 😄