AbsaOSS / AbsaOSS/EventGate

Remove __all__ declarations from src/utils modules

Aberta
#221 0 comentários 0 reações 0 responsáveis Ver no GitHub
refactoring type:tech-debt
Linguagem predominante
Python
Estrelas
4
Forks
0
Merge médio
1d 5h
PRs com merge (30d)
8

Descrição

### 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.

Guia de contribuição

Nenhum guia de contribuição indexado para este repositório

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.