kianiadee/tracksolid_timescale_grafana_prod
1
0
Fork 0
Code Issues Pull requests 4 Projects Releases Packages Wiki Activity Actions 7

fix: BUG-06..11 — pool lock, rounding, tz, infer_city, rowcount, double commit #14

Open
kianiadee wants to merge 1 commit from fix/bugs-06-to-11 into main
pull from: fix/bugs-06-to-11
merge into: kianiadee:main
Conversation 0 Commits 1 Files changed 4 +94 -24
kianiadee commented 2026-05-15 12:50:29 +00:00
Owner
Copy link

Summary

Third and final tranche of audit fixes — the LOW + COSMETIC batch. Independent of PR #12 and PR #13 (no overlapping lines).

  • BUG-06 (LOW-MED): _get_pool() had a TOCTOU race on cold start. Two threads racing through the None-check could each instantiate a ThreadedConnectionPool; the second assignment overwrote the first and leaked its checked-out connections. Added threading.Lock with a double-checked-locking pattern.
  • BUG-07 (LOW): clean_int used int(float(s)) which truncates ("3.9" → 3). Switched to round(...). All current call sites pass intrinsically-integer fields so production behaviour is unchanged; rounding is a safer default for future float-valued integer fields. Unit test updated.
  • BUG-08 (LOW): _infer_city mapped every Kenyan plate to NBO, silently misclassifying Coast vehicles. Now returns None for K-series plates and logs a warning so operators can tag them explicitly. Uganda (UMA/UAG → KLA) is unchanged. Analytics views already COALESCE(...) NULL assigned_city into the unassigned bucket, so no dashboard panels break.
  • BUG-09 (LOW): clean_ts("2024-04-12") returned the date verbatim, which Postgres interprets as 00:00 UTC = 03:00 EAT — three hours off the operator's intent. Date-only strings are now anchored to Africa/Nairobi midnight (T00:00:00+03:00). Inputs with a time component pass through unchanged. Unit test added.
  • BUG-10 (LOW): rowcount counters in poll_live_positions and poll_trips were named upserted/inserted but accumulated cur.rowcount from ON CONFLICT DO UPDATE statements — always 1 regardless of insert vs. update. Renamed to live_pos_affected / history_inserted / trips_affected, and routed trips_affected to the rows_upserted slot of ingestion_log (it was previously logged as rows_inserted).
  • BUG-11 (COSMETIC): removed redundant explicit conn.commit() inside the with get_conn() block of _update_token_cache — the context manager auto-commits on __exit__.

Test plan

  • CI / pytest tests/unit/test_clean_helpers.py passes (covers BUG-07 + BUG-09 directly).
  • Deploy. Both ingest containers and the webhook receiver restart cleanly.
  • Confirm ingestion_log rows for jimi.device.track.mileage start populating rows_upserted instead of rows_inserted.
  • Run python import_drivers_csv.py and confirm K-series plates emit the new "Kenyan prefix is ambiguous" warning and resolve to assigned_city IS NULL.
  • Confirm tracksolid.api_token_cache still updates correctly (token cache write is the BUG-11 code path).
  • Sanity-check the Daily Operations dashboard city filter still works — unassigned should now contain the Kenyan vehicles.

🤖 Generated with Claude Code

## Summary Third and final tranche of audit fixes — the LOW + COSMETIC batch. Independent of PR #12 and PR #13 (no overlapping lines). - **BUG-06 (LOW-MED):** `_get_pool()` had a TOCTOU race on cold start. Two threads racing through the `None`-check could each instantiate a `ThreadedConnectionPool`; the second assignment overwrote the first and leaked its checked-out connections. Added `threading.Lock` with a double-checked-locking pattern. - **BUG-07 (LOW):** `clean_int` used `int(float(s))` which truncates (`"3.9" → 3`). Switched to `round(...)`. All current call sites pass intrinsically-integer fields so production behaviour is unchanged; rounding is a safer default for future float-valued integer fields. Unit test updated. - **BUG-08 (LOW):** `_infer_city` mapped every Kenyan plate to `NBO`, silently misclassifying Coast vehicles. Now returns `None` for K-series plates and logs a warning so operators can tag them explicitly. Uganda (UMA/UAG → KLA) is unchanged. Analytics views already `COALESCE(...)` NULL `assigned_city` into the `unassigned` bucket, so no dashboard panels break. - **BUG-09 (LOW):** `clean_ts("2024-04-12")` returned the date verbatim, which Postgres interprets as `00:00 UTC = 03:00 EAT` — three hours off the operator's intent. Date-only strings are now anchored to Africa/Nairobi midnight (`T00:00:00+03:00`). Inputs with a time component pass through unchanged. Unit test added. - **BUG-10 (LOW):** `rowcount` counters in `poll_live_positions` and `poll_trips` were named `upserted`/`inserted` but accumulated `cur.rowcount` from `ON CONFLICT DO UPDATE` statements — always 1 regardless of insert vs. update. Renamed to `live_pos_affected` / `history_inserted` / `trips_affected`, and routed `trips_affected` to the `rows_upserted` slot of `ingestion_log` (it was previously logged as `rows_inserted`). - **BUG-11 (COSMETIC):** removed redundant explicit `conn.commit()` inside the `with get_conn()` block of `_update_token_cache` — the context manager auto-commits on `__exit__`. ## Test plan - [ ] CI / `pytest tests/unit/test_clean_helpers.py` passes (covers BUG-07 + BUG-09 directly). - [ ] Deploy. Both ingest containers and the webhook receiver restart cleanly. - [ ] Confirm `ingestion_log` rows for `jimi.device.track.mileage` start populating `rows_upserted` instead of `rows_inserted`. - [ ] Run `python import_drivers_csv.py` and confirm K-series plates emit the new "Kenyan prefix is ambiguous" warning and resolve to `assigned_city IS NULL`. - [ ] Confirm `tracksolid.api_token_cache` still updates correctly (token cache write is the BUG-11 code path). - [ ] Sanity-check the Daily Operations dashboard `city` filter still works — `unassigned` should now contain the Kenyan vehicles. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
kianiadee added 1 commit 2026-05-15 12:50:29 +00:00
fix: BUG-06..11 — pool lock, clean_int rounding, date-only tz, _infer_city, rowcount naming, double commit
Some checks failed
Static Analysis / static (push) Has been cancelled
Details
Tests / test (push) Has been cancelled
Details
Static Analysis / static (pull_request) Has been cancelled
Details
Tests / test (pull_request) Has been cancelled
Details
d66c3bab42 
BUG-06 (LOW-MED): _get_pool() had a TOCTOU race — two threads hitting the
None pool at cold start could each create one and leak the loser's
connections. Added a threading.Lock with double-checked locking.

BUG-07 (LOW): clean_int truncated via int(float(s)) so "3.9" → 3. All
current call sites are intrinsically-integer fields, so behaviour for
production traffic is unchanged, but rounding is the safer default for
any future field that arrives as a decimal. Unit test updated to match.

BUG-08 (LOW): _infer_city mapped every Kenyan plate to NBO, silently
misclassifying Coast/Mombasa vehicles. Now returns None for K-series
plates and emits a log warning so operators can tag them explicitly.
Uganda (UMA / UAG) remains unambiguous → KLA. Analytics views already
COALESCE NULLs into the 'unassigned' bucket so no dashboards break.

BUG-09 (LOW): clean_ts accepted "2024-04-12" verbatim → Postgres stored
00:00 UTC = 03:00 EAT, three hours off the operator's intent. Date-only
strings are now anchored to Africa/Nairobi midnight (T00:00:00+03:00).
Strings with a time component pass through unchanged. Unit test added.

BUG-10 (LOW): rowcount counters in poll_live_positions and poll_trips
were named "upserted"/"inserted" but they sum cur.rowcount from
ON CONFLICT DO UPDATE statements — which always returns 1 per touch
regardless of whether the row was an insert or an update. Renamed to
live_pos_affected / history_inserted / trips_affected, and routed
trips_affected to the rows_upserted slot of ingestion_log (it was
previously logged as rows_inserted, which was misleading).

BUG-11 (COSMETIC): removed the redundant conn.commit() inside the
with get_conn() block of _update_token_cache — the context manager
already auto-commits on __exit__.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Some checks failed
Static Analysis / static (push) Has been cancelled
Details
Tests / test (push) Has been cancelled
Details
Static Analysis / static (pull_request) Has been cancelled
Details
Tests / test (pull_request) Has been cancelled
Details
This pull request has changes conflicting with the target branch.
  • import_drivers_csv.py
  • ingest_movement_rev.py
  • ts_shared_rev.py
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/bugs-06-to-11:fix/bugs-06-to-11
git checkout fix/bugs-06-to-11

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff fix/bugs-06-to-11
git checkout main
git merge --ff-only fix/bugs-06-to-11
git checkout fix/bugs-06-to-11
git rebase main
git checkout main
git merge --no-ff fix/bugs-06-to-11
git checkout main
git merge --squash fix/bugs-06-to-11
git checkout main
git merge --ff-only fix/bugs-06-to-11
git checkout main
git merge fix/bugs-06-to-11
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: kianiadee/tracksolid_timescale_grafana_prod#14
No description provided.
Delete branch "fix/bugs-06-to-11"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?