AbsaOSS / AbsaOSS/EventGate

Remove __all__ declarations from src/utils modules

Aperta
#221 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
refactoring type:tech-debt
Lingua principale
Python
Stelle
4
Fork
0
Merge medio
1g 5h
PR unite (30g)
8

Descrizione

### Description of Technical Debt

Five modules under `src/utils/` end with an `__all__` declaration:

| Module | Line | Names |
|---|---|---|
| `src/utils/conf_path.py` | 90 | 4 |
| `src/utils/logging_levels.py` | 71 | 3 |
| `src/utils/observability.py` | 217 | 16 |
| `src/utils/safe_serialization.py` | 87 | 1 |
| `src/utils/trace_logging.py` | 53 | 1 |

`__all__` controls exactly one thing: which names `from module import *` binds. There is no `import *` anywhere in `src/` or `tests/`, and explicit imports are the convention the codebase already follows everywhere. The declarations therefore have no effect on any current call site.

Raised in review on PR #204 and agreed by both reviewer and author.

### Impact of Technical Debt

- **Maintenance with no return.** Every new public name in these modules means editing a second list that nothing reads. The lists are in sync today, so the cost so far has been paid entirely in review attention rather than caught bugs.
- **Misleading signal.** A reader encountering `__all__` reasonably infers that star imports are expected somewhere, or that the module has a curated public surface distinct from its non-underscore names. Neither is true.
- **Inconsistent with the rest of the codebase.** No other package in the project declares `__all__`, and it is not a pattern used in sibling Python projects, so it reads as local convention where there is none.

### Category

Code Quality / Refactoring

### Priority

Low - Nice to have

### Proposed Solution

Delete the `__all__` block from all five modules. Rely on the existing convention: a leading underscore marks a name private, everything else is importable.

Two things to confirm while doing it, both currently safe:

1. **Re-exports.** `src/utils/observability.py` lists `TRACE_LEVEL` and `configured_log_level`, which it imports from `src/utils/logging_levels.py` rather than defining; `src/utils/conf_path.py` similarly lists `CONF_DIR` and `INVALID_CONF_ENV`. Under mypy's `no_implicit_reexport` an imported name is only re-exported if it appears in `__all__` or is aliased. The project's mypy config (`pyproject.toml [tool.mypy]`) does **not** enable `strict` or `no_implicit_reexport`, so removal changes nothing — but if that config is tightened later, importers of those names through `observability` would need to import them from `logging_levels` instead.
2. **No star imports.** `grep -rn "import \*" --include=*.py src/ tests/` returns nothing. Re-run before merging in case one has been added.

### Effort Estimate

< 1 hour

### Dependencies / Related

- PR #204, where this was raised
- #193

### Additional Context

Acceptance:

- No `__all__` remains under `src/`.
- `pytest tests/unit`, `mypy`, `pylint`, and `black --check` all pass unchanged.
- No import statement anywhere needs editing, which is the point: if one does, that name was relying on a re-export and should be imported from its defining module.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.