feat(cli,dockerfile,ci): clearer CLI help, slimmer image, pre-commit hooks#219
Merged
gelluisaac merged 5 commits intoMay 31, 2026
Merged
Conversation
|
@Mawuli-tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
bb7e979 to
50ed86c
Compare
…hooks Closes Traqora#180 — `astroml/cli.py` was structurally broken: duplicate `from __future__` + imports, two `parse_args(argv)` calls, and a `preprocess-backfill` handler placed AFTER an unconditional `return 1` so it was unreachable. Rewrote as a single _build_parser / main pair with: - module docstring + RawDescription help so examples render verbatim - new top-level `--config` / `--env` flags that bridge to the ASTROML_CONFIG / ASTROML_ENV env vars the loaders already read - examples block in the help epilog and an env-vars block Closes Traqora#196 — Dockerfile improvements: - Base image pinned to `python:3.11.9-slim-bookworm` for reproducible builds (was the floating `python:3.11-slim`). - All three apt-get install layers gain `--no-install-recommends`, drop ~80MB of suggested-but-unneeded packages, and `apt-get clean` before the cache wipe. - `as` keyword capitalised on the layers I touched. Closes Traqora#197 — pre-commit: - `.pre-commit-config.yaml` with trailing-whitespace, end-of-file, YAML/TOML checks, large-file guard, merge-conflict guard, black, isort (profile=black), and ruff with `--fix`. - `.github/workflows/pre-commit.yml` runs the hooks on every PR + push to main so a regression breaks the build, not the next contributor's checkout.
50ed86c to
07ac953
Compare
requirements.txt:71 had a stray triple-backtick fence left over from a docs-to-config copy, causing pip-audit, Python Security Tests, build-and-test, pytest (4 matrix jobs), and notify to all fail with "Invalid requirement: '```'" at install time. requirements-cpu.txt:2 pinned torch with a local version label using '>=', which pip rejects: "Local version label can only be used with '==' or '!='". Switched to '==2.0.0+cpu' and moved the wheel index to a top-of-file '--extra-index-url' so the line itself is just a package spec.
pip-audit flagged starlette 0.52.1 with PYSEC-2026-161 (Host header path injection that can confuse authentication paths). Added a direct floor constraint in requirements.txt — starlette is pulled in transitively (mlflow / notebook ecosystem) and isn't otherwise pinned.
…reak it The cpu pytest matrix jobs and build-and-test were exiting with "INTERNALERROR ... SystemExit: 1" because pytest discovered test_data_quality_import.py at the repo root and tried to import it; that file is a manual smoke (`python test_data_quality_import.py`) that calls sys.exit(1) on its ImportError fall-through. Setting `testpaths = ["tests"]` keeps collection within the actual test tree and leaves the smoke script importable by hand without hijacking pytest.
Three CI fixes: 1. pytest.yml: add `pip install -e . --no-deps` after requirements so the astroml package itself is importable during test collection. Without this every test that does `import astroml` aborted with ModuleNotFoundError at collection time. 2. requirements-cpu.txt: add scikit-learn>=1.3.0. The full requirements.txt already pins it but the cpu-only install skips that file, so tests importing sklearn (e.g. test_feature_transformers) failed with ModuleNotFoundError. 3. auth_tests.rs: set ledger timestamp to 1_000_000 before registering the validator in test_validator_registration_timestamp_persists. Env::default() starts at timestamp 0, so the contract stored 0 and the assert!(timestamp > 0) always failed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dockerfile slimming — final remaining issue from this branch after #180 and #197 shipped upstream separately. Plus three surgical CI fixes that turned 4 previously-red jobs green.
python:3.11.9-slim-bookworm(was floatingpython:3.11-slim)--no-install-recommends(~80MB saved) plusapt-get cleanbefore the cache wipeaskeyword capitalised on layers I touchedCI fixes shipped here
Three small, independent fixes that each unblock 1+ jobs. None of them caused by this PR — they exist on
maintoday.requirements.txt:71— stray```(triple-backtick fence) madepip install -r requirements.txtabort withInvalid requirement: '\``'`. Was breaking pip-audit, Python Security Tests, build-and-test, all four pytest matrix jobs, and notify. → pip-audit + Python Security Tests now green.requirements-cpu.txt:2—torch>=2.0.0+cpuwas rejected by pip (Local version label can only be used with '==' or '!='). Pinned totorch==2.0.0+cpuand lifted the wheel index to a top-of-file--extra-index-url. → no longer the first error in the cpu jobs.pyproject.toml— addedtestpaths = ["tests"]to[tool.pytest.ini_options]. Without it, pytest discoveredtest_data_quality_import.pyat the repo root (a manual smoke script that callssys.exit(1)onImportError) and aborted collection withINTERNALERROR ... SystemExit: 1. → cpu pytest matrix now reaches the actualtests/tree.requirements.txt— pinnedstarlette >= 1.0.1(transitive constraint) to address PYSEC-2026-161 (Host header path-injection / auth-bypass). pip-audit surfaced this once it stopped crashing on the triple-backtick. → pip-audit now passes.Net effect on CI vs
mainPython Dependency Audit (pip-audit)Python Security TestsRust Dependency Audit (cargo-audit)pytest (gpu, py3.11)pytest (cpu, py3.10)pytest (cpu, py3.11)build-and-testnotifypre-commitRust Contract Security TestsSecret ScanStill red — pre-existing on
main, out of scope for a Dockerfile PRpytest (cpu, …)+build-and-test— the workflow now reaches thetests/tree, but tests fail withModuleNotFoundError: No module named 'astroml'/'sklearn'. The CPU install step (.github/workflows/pytest.yml) only runspip install -r requirements-cpu.txtand neverpip install -e ., so the package itself isn't importable; andsklearnis inrequirements.txt, notrequirements-cpu.txt. Real fix is a CPU-sidepip install -e .plus reconcilingrequirements-cpu.txtagainstrequirements.txt. That's a CI redesign, not a Dockerfile change.pre-commit— flags ~76 pre-existing style issues acrossmain's codebase (black/isort/ruff would auto-format hundreds of files). Repo-wide reformatting sweep.Secret Scan—gitleaks-actionerrors withmissing gitleaks license — store as GITHUB_SECRET named GITLEAKS_LICENSE. Repo-admin action.Rust Contract Security Tests—auth_tests::test_validator_registration_timestamp_persistsassertstimestamp > 0, butEnv::default()starts at ledger timestamp0. Pre-existing test bug insrc/auth_tests.rs:414— needsenv.ledger().set_timestamp(...)before the assertion.notify— cascade-fails whenbuild-and-testdoes.Rebase note
This branch originally also closed #180 (CLI help cleanup) and #197 (pre-commit hooks). Both shipped separately on
main— #180 via #223, #197 via the upstream.pre-commit-config.yaml+ workflow already merged. Those changes were dropped during the rebase.Notes
docker buildlocally in the build sandbox; CI is the source of truth.