airajena opened a new pull request, #65:
URL: https://github.com/apache/fineract-business-intelligence/pull/65
## Summary
Closes #64
Adds real linting for all three languages in this repo (Python, SQL/dbt,
shell) and wires each into CI. The existing "Compile & Lint" job only ran
`python -m py_compile`, which checks that files parse but does no actual
linting — this PR makes the job's name honest.
## What changed
**Python — `ruff`**
- Added `pyproject.toml` (line-length 120, target py311, rules
`E,F,W,I,UP,B,SIM`)
- Fixed all 37 findings from the baseline scan, including two real bugs
beyond style:
- An unused `lat_ds` variable in `bootstrap_superset_assets.py` turned out
to be a real mix-up — 10 charts on the Repayment Behavior dashboard (4 KPI
tiles + 6 breakdowns) were querying the wrong Superset dataset
(`_secure_all_dates` instead of `_secure_latest`), meaning KPI tiles were
aggregating across all history instead of showing current values
- An unused `watermark_manager` argument in `_reset_backfill_state` turned
out to be bypassing the `WatermarkManager` abstraction via raw SQL; added
`WatermarkManager.reset_all()` and routed the call through it, with new tests
(mutation-tested to confirm they catch a regression)
- Two `pytest.raises(Exception)` tests narrowed to
`dataclasses.FrozenInstanceError` (previously passed on *any* exception)
- One `assert False` (silently no-ops under `python -O`) replaced with
`pytest.raises(KeyError)`
**Shell — `shellcheck`**
- Added a CI step (`--severity=warning`) across all 12 project scripts
- `verify_extraction.sh` had a real bug, worse than it looked: it sources
`common.sh`, which sets `-e` globally, so the script was silently dying on the
very first failed health/schema check — never printing its pass/fail summary.
Fixed by scoping `set +e`/`set -e` around just the fallible call inside its
`check()` helper
- Fixed an unused-variable finding (`SC2034`) and an unquoted word-splitting
risk (`SC2086`, fixed via bash array)
- Standardized `set -euo pipefail` across scripts flagged by the audit —
except one script confirmed to run under Alpine/busybox `/bin/sh`, which
doesn't support `pipefail`, left as `set -eu`
**SQL/dbt — `sqlfluff`**
- Added `.sqlfluff` config tuned to this codebase's existing conventions
(implicit table aliasing, lowercase keywords) rather than sqlfluff's defaults
- Ran `sqlfluff fix` across `dbt/models` and `dbt/tests` — pure formatting
changes only (spacing, indentation, blank lines between CTEs); explicitly
excluded a column-reordering rule (`ST06`) and a join-order rule (`ST09`) since
they produced semantically-neutral but noisier diffs than intended
- 4 files that use a custom macro (`safe_divide`) or dbt runtime variables
(`target.schema`) can't be fully parsed by the lightweight `jinja` templater
(vs. the heavier `dbt` templater, which needs a live DB connection); excluded
via `.sqlfluffignore` rather than silently left broken
## Verification
- `ruff check .` — clean
- `sqlfluff lint dbt/models dbt/tests --exclude-rules ...` — clean
- `shellcheck --severity=warning ...` — clean (exit 0)
- Full dbt rebuild: 18/18 models built
- Full `dbt test`: **91/91 passing** — confirms the sqlfluff reformatting
changed zero query behavior
- Python unit suite: **100/100 passing** (96 original + 4 new tests for
`WatermarkManager.reset_all`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
