feat: basketball-api HTTP client with Keycloak auth #11

Merged

forgejo_admin merged 1 commit from 5-basketball-client into main 2026-03-28 22:43:59 +00:00

Conversation 2 Commits 1 Files changed 2 +655

forgejo_admin commented 2026-03-28 22:40:32 +00:00

Owner

Copy link

Summary

Async httpx client that authenticates to basketball-api via Keycloak client credentials flow, caches tokens with 30s pre-expiry refresh, and provides typed functions for all 14 basketball-api endpoints (7 reads, 7 writes). Tenant slug hardcoded to "westside-kings-queens".

Changes

• app/basketball.py -- new BasketballClient class with:
  • Keycloak client credentials token acquisition (POST to {KEYCLOAK_REALM_URL}/protocol/openid-connect/token)
  • Token caching with auto-refresh 30s before expiry
  • Bearer token on all requests via _auth_headers()
  • 7 read functions: get_dashboard, list_players, get_player, list_teams, get_roster, get_subscriptions_overview, list_subscriptions
  • 7 write functions: update_player, assign_player_to_team, remove_player_from_team, toggle_player_visibility, create_team, checkin_player, bulk_assign_tryout_numbers
  • BasketballAPIError exception with status code and body
• tests/test_basketball.py -- 22 unit tests covering:
  • Token acquisition, caching, and 30s pre-expiry refresh
  • Keycloak error handling
  • Bearer header presence on all requests
  • HTTP error propagation (500, 404)
  • Every read operation verifies correct HTTP method + path
  • Every write operation verifies correct HTTP method + path + JSON body

Test Plan

pytest tests/ -v
# 30 passed (22 new basketball + 8 existing groupme/health)

Review Checklist

• Token caching with 30s pre-expiry refresh
• All 14 endpoints match the issue reference table exactly
• Bearer header on every request
• HTTP errors raised as BasketballAPIError with status + body
• No modifications to basketball-api or other repos
• No modifications to app/groupme.py (wiring is #6)
• All 30 tests pass (22 new + 8 existing)

Related Notes

• None (client-only module, no pal-e-docs notes)

Related

Closes #5

• Depends on: #4 (scaffold)
• Keycloak client provisioned by: #1

## Summary Async httpx client that authenticates to basketball-api via Keycloak client credentials flow, caches tokens with 30s pre-expiry refresh, and provides typed functions for all 14 basketball-api endpoints (7 reads, 7 writes). Tenant slug hardcoded to "westside-kings-queens". ## Changes - **`app/basketball.py`** -- new `BasketballClient` class with: - Keycloak client credentials token acquisition (POST to `{KEYCLOAK_REALM_URL}/protocol/openid-connect/token`) - Token caching with auto-refresh 30s before expiry - Bearer token on all requests via `_auth_headers()` - 7 read functions: `get_dashboard`, `list_players`, `get_player`, `list_teams`, `get_roster`, `get_subscriptions_overview`, `list_subscriptions` - 7 write functions: `update_player`, `assign_player_to_team`, `remove_player_from_team`, `toggle_player_visibility`, `create_team`, `checkin_player`, `bulk_assign_tryout_numbers` - `BasketballAPIError` exception with status code and body - **`tests/test_basketball.py`** -- 22 unit tests covering: - Token acquisition, caching, and 30s pre-expiry refresh - Keycloak error handling - Bearer header presence on all requests - HTTP error propagation (500, 404) - Every read operation verifies correct HTTP method + path - Every write operation verifies correct HTTP method + path + JSON body ## Test Plan ``` pytest tests/ -v # 30 passed (22 new basketball + 8 existing groupme/health) ``` ## Review Checklist - [x] Token caching with 30s pre-expiry refresh - [x] All 14 endpoints match the issue reference table exactly - [x] Bearer header on every request - [x] HTTP errors raised as `BasketballAPIError` with status + body - [x] No modifications to basketball-api or other repos - [x] No modifications to `app/groupme.py` (wiring is #6) - [x] All 30 tests pass (22 new + 8 existing) ## Related Notes - None (client-only module, no pal-e-docs notes) ## Related Closes #5 - Depends on: #4 (scaffold) - Keycloak client provisioned by: #1

forgejo_admin added 1 commit 2026-03-28 22:40:32 +00:00

feat: basketball-api HTTP client with Keycloak client credentials auth ... 2750dd8a9c

Async httpx client that authenticates via Keycloak client credentials
flow, caches tokens with 30s pre-expiry refresh, and provides typed
functions for all 14 basketball-api endpoints (7 reads, 7 writes).
Tenant slug hardcoded to westside-kings-queens.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-28 22:41:20 +00:00

Author

Owner

Copy link

QA Review -- PR #11

Endpoint Verification (cross-referenced against issue #5 table)

Reads (7/7 match):

Function	Expected	Actual	Status
get_dashboard	GET /admin/dashboard	GET /admin/dashboard	OK
list_players	GET /admin/players	GET /admin/players	OK
get_player	GET /players/{player_id}	GET /players/{player_id}	OK
list_teams	GET /admin/teams	GET /admin/teams	OK
get_roster	GET /tenants/westside-kings-queens/roster	GET /tenants/westside-kings-queens/roster	OK
get_subscriptions_overview	GET /api/subscriptions/overview	GET /api/subscriptions/overview	OK
list_subscriptions	GET /api/subscriptions	GET /api/subscriptions	OK

Writes (7/7 match):

Function	Expected	Actual	Status
update_player	PUT /players/{player_id}	PUT /players/{player_id}	OK
assign_player_to_team	POST /teams/{team_id}/players	POST /teams/{team_id}/players	OK
remove_player_from_team	DELETE /teams/{team_id}/players/{player_id}	DELETE /teams/{team_id}/players/{player_id}	OK
toggle_player_visibility	PATCH /admin/players/{player_id}/visibility	PATCH /admin/players/{player_id}/visibility	OK
create_team	POST /api/teams	POST /api/teams	OK
checkin_player	POST /api/roster/westside-kings-queens/check-in/{player_id}	POST /api/roster/westside-kings-queens/check-in/{player_id}	OK
bulk_assign_tryout_numbers	POST /tryouts/admin/westside-kings-queens/assign-numbers	POST /tryouts/admin/westside-kings-queens/assign-numbers	OK

Acceptance Criteria

• Client authenticates via Keycloak client credentials POST to {KEYCLOAK_REALM_URL}/protocol/openid-connect/token
• Token cached, auto-refreshed 30s before expiry
• All functions include Bearer token in Authorization header
• HTTP errors raised as exceptions with status code and body (BasketballAPIError)
• Tenant slug hardcoded to "westside-kings-queens" (TENANT_SLUG constant)

Code Quality

• Clean separation: token management, request helper, read ops, write ops
• Constructor accepts overrides for all config (enables testability without monkeypatching settings)
• Lazy httpx client creation with proper close() cleanup
• 204/empty body handling in _request()
• Tests use httpx.Response mock objects (not dict stubs) -- correct abstraction level
• 22 tests cover: token acquire, cache, refresh, Keycloak error, bearer header, HTTP errors, all 14 endpoints

Boundary Check

• No modifications to basketball-api
• No modifications to app/groupme.py
• No modifications to requirements.txt (httpx already present from #4)
• Only 2 new files: app/basketball.py, tests/test_basketball.py

Nits

None.

VERDICT: APPROVED

## QA Review -- PR #11 ### Endpoint Verification (cross-referenced against issue #5 table) **Reads (7/7 match):** | Function | Expected | Actual | Status | |----------|----------|--------|--------| | get_dashboard | GET /admin/dashboard | GET /admin/dashboard | OK | | list_players | GET /admin/players | GET /admin/players | OK | | get_player | GET /players/{player_id} | GET /players/{player_id} | OK | | list_teams | GET /admin/teams | GET /admin/teams | OK | | get_roster | GET /tenants/westside-kings-queens/roster | GET /tenants/westside-kings-queens/roster | OK | | get_subscriptions_overview | GET /api/subscriptions/overview | GET /api/subscriptions/overview | OK | | list_subscriptions | GET /api/subscriptions | GET /api/subscriptions | OK | **Writes (7/7 match):** | Function | Expected | Actual | Status | |----------|----------|--------|--------| | update_player | PUT /players/{player_id} | PUT /players/{player_id} | OK | | assign_player_to_team | POST /teams/{team_id}/players | POST /teams/{team_id}/players | OK | | remove_player_from_team | DELETE /teams/{team_id}/players/{player_id} | DELETE /teams/{team_id}/players/{player_id} | OK | | toggle_player_visibility | PATCH /admin/players/{player_id}/visibility | PATCH /admin/players/{player_id}/visibility | OK | | create_team | POST /api/teams | POST /api/teams | OK | | checkin_player | POST /api/roster/westside-kings-queens/check-in/{player_id} | POST /api/roster/westside-kings-queens/check-in/{player_id} | OK | | bulk_assign_tryout_numbers | POST /tryouts/admin/westside-kings-queens/assign-numbers | POST /tryouts/admin/westside-kings-queens/assign-numbers | OK | ### Acceptance Criteria - [x] Client authenticates via Keycloak client credentials POST to `{KEYCLOAK_REALM_URL}/protocol/openid-connect/token` - [x] Token cached, auto-refreshed 30s before expiry - [x] All functions include Bearer token in Authorization header - [x] HTTP errors raised as exceptions with status code and body (`BasketballAPIError`) - [x] Tenant slug hardcoded to "westside-kings-queens" (`TENANT_SLUG` constant) ### Code Quality - Clean separation: token management, request helper, read ops, write ops - Constructor accepts overrides for all config (enables testability without monkeypatching settings) - Lazy httpx client creation with proper `close()` cleanup - 204/empty body handling in `_request()` - Tests use httpx.Response mock objects (not dict stubs) -- correct abstraction level - 22 tests cover: token acquire, cache, refresh, Keycloak error, bearer header, HTTP errors, all 14 endpoints ### Boundary Check - [x] No modifications to basketball-api - [x] No modifications to `app/groupme.py` - [x] No modifications to `requirements.txt` (httpx already present from #4) - [x] Only 2 new files: `app/basketball.py`, `tests/test_basketball.py` ### Nits None. ### VERDICT: APPROVED

forgejo_admin commented 2026-03-28 22:43:16 +00:00

Author

Owner

Copy link

PR #11 Review

DOMAIN REVIEW

Tech stack: Python / async httpx / Keycloak OAuth2 client credentials / pytest-asyncio

Token management -- The 30s pre-expiry refresh logic in _ensure_token is correct. The fallback expires_in=300 on payload.get("expires_in", 300) is a reasonable default. Token is cached on the instance and reused across requests. Each token acquisition creates a scoped async with httpx.AsyncClient() that is properly closed.

HTTP client lifecycle -- _get_http() lazily creates the API client; close() properly calls aclose() and resets to None. Clean resource management.

Error handling -- BasketballAPIError carries both status_code and body, and _request correctly raises on any non-2xx status. 204/empty-content responses are handled by returning None. Keycloak token errors are also raised as BasketballAPIError, which is a good unified error surface.

14 endpoints -- All 7 reads and 7 writes are thin wrappers over _request with the correct HTTP method and path. Tenant slug is centralized in TENANT_SLUG constant and used consistently in get_roster, checkin_player, and bulk_assign_tryout_numbers.

Test coverage -- 22 tests across 5 classes. Token lifecycle is well-tested (acquisition, caching, pre-expiry refresh, Keycloak error). Every endpoint is individually tested for correct HTTP method, path, and JSON body. Bearer header presence is verified. HTTP error propagation covers 500 and 404. This is solid coverage.

No secrets in code -- All credentials are sourced from settings (via app.config) or constructor arguments. No hardcoded API keys, passwords, or tokens.

BLOCKERS

None.

NITS

1. Concurrent token refresh race (app/basketball.py:67-85): If two coroutines hit _ensure_token concurrently when the token is expired, both will POST to Keycloak. Not a correctness bug (both get valid tokens), but wasteful. An asyncio.Lock around the refresh block would prevent duplicate requests. Low priority for a single-consumer bot.

2. Type hints on write methods (app/basketball.py:153,158,175): data: dict lacks key/value type annotation. PEP 484 would prefer dict[str, Any] for clarity. The -> Any return types on all methods are also quite broad -- consider defining response TypedDicts for the most-used endpoints as the codebase matures.

3. Missing test for close() behavior (tests/test_basketball.py): No test verifies that close() properly resets the internal HTTP client. Minor, since the fixture exercises it implicitly.

4. Missing test for 204 on a read path (tests/test_basketball.py): The 204/empty-content branch in _request is only exercised via test_remove_player_from_team (a write). No read test covers the edge case where a read endpoint returns 204. Low risk since the code path is shared, but worth a quick test for completeness.

5. _token type assertion (app/basketball.py:82): self._token = payload["access_token"] -- if the Keycloak response is missing access_token, this raises a raw KeyError. Consider catching this and raising BasketballAPIError with a descriptive message instead.

SOP COMPLIANCE

• Branch named after issue (5-basketball-client matches issue #5)
• PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related)
• Related references parent issue (Closes #5) and dependencies (#4, #1)
• No secrets committed
• No scope creep (exactly 2 new files, no modifications to existing code)
• Commit message is descriptive (feat: basketball-api HTTP client with Keycloak auth)

PROCESS OBSERVATIONS

Clean, focused PR. Two new files, zero modifications to existing code. The 22 tests are well-structured and cover the full endpoint surface plus auth lifecycle. The separation between this client module (#5) and the wiring/integration (#6) shows good decomposition discipline. No DORA risk -- this is additive code with no deployment-time state changes.

VERDICT: APPROVED

## PR #11 Review ### DOMAIN REVIEW **Tech stack**: Python / async httpx / Keycloak OAuth2 client credentials / pytest-asyncio **Token management** -- The 30s pre-expiry refresh logic in `_ensure_token` is correct. The fallback `expires_in=300` on `payload.get("expires_in", 300)` is a reasonable default. Token is cached on the instance and reused across requests. Each token acquisition creates a scoped `async with httpx.AsyncClient()` that is properly closed. **HTTP client lifecycle** -- `_get_http()` lazily creates the API client; `close()` properly calls `aclose()` and resets to `None`. Clean resource management. **Error handling** -- `BasketballAPIError` carries both `status_code` and `body`, and `_request` correctly raises on any non-2xx status. 204/empty-content responses are handled by returning `None`. Keycloak token errors are also raised as `BasketballAPIError`, which is a good unified error surface. **14 endpoints** -- All 7 reads and 7 writes are thin wrappers over `_request` with the correct HTTP method and path. Tenant slug is centralized in `TENANT_SLUG` constant and used consistently in `get_roster`, `checkin_player`, and `bulk_assign_tryout_numbers`. **Test coverage** -- 22 tests across 5 classes. Token lifecycle is well-tested (acquisition, caching, pre-expiry refresh, Keycloak error). Every endpoint is individually tested for correct HTTP method, path, and JSON body. Bearer header presence is verified. HTTP error propagation covers 500 and 404. This is solid coverage. **No secrets in code** -- All credentials are sourced from `settings` (via `app.config`) or constructor arguments. No hardcoded API keys, passwords, or tokens. ### BLOCKERS None. ### NITS 1. **Concurrent token refresh race** (`app/basketball.py:67-85`): If two coroutines hit `_ensure_token` concurrently when the token is expired, both will POST to Keycloak. Not a correctness bug (both get valid tokens), but wasteful. An `asyncio.Lock` around the refresh block would prevent duplicate requests. Low priority for a single-consumer bot. 2. **Type hints on write methods** (`app/basketball.py:153,158,175`): `data: dict` lacks key/value type annotation. PEP 484 would prefer `dict[str, Any]` for clarity. The `-> Any` return types on all methods are also quite broad -- consider defining response TypedDicts for the most-used endpoints as the codebase matures. 3. **Missing test for `close()` behavior** (`tests/test_basketball.py`): No test verifies that `close()` properly resets the internal HTTP client. Minor, since the fixture exercises it implicitly. 4. **Missing test for 204 on a read path** (`tests/test_basketball.py`): The 204/empty-content branch in `_request` is only exercised via `test_remove_player_from_team` (a write). No read test covers the edge case where a read endpoint returns 204. Low risk since the code path is shared, but worth a quick test for completeness. 5. **`_token` type assertion** (`app/basketball.py:82`): `self._token = payload["access_token"]` -- if the Keycloak response is missing `access_token`, this raises a raw `KeyError`. Consider catching this and raising `BasketballAPIError` with a descriptive message instead. ### SOP COMPLIANCE - [x] Branch named after issue (`5-basketball-client` matches issue #5) - [x] PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related) - [x] Related references parent issue (`Closes #5`) and dependencies (#4, #1) - [x] No secrets committed - [x] No scope creep (exactly 2 new files, no modifications to existing code) - [x] Commit message is descriptive (`feat: basketball-api HTTP client with Keycloak auth`) ### PROCESS OBSERVATIONS Clean, focused PR. Two new files, zero modifications to existing code. The 22 tests are well-structured and cover the full endpoint surface plus auth lifecycle. The separation between this client module (#5) and the wiring/integration (#6) shows good decomposition discipline. No DORA risk -- this is additive code with no deployment-time state changes. ### VERDICT: APPROVED

forgejo_admin merged commit 72d195fecb into main 2026-03-28 22:43:59 +00:00

forgejo_admin deleted branch 5-basketball-client 2026-03-28 22:44:00 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-28 22:44:01 +00:00

feat: basketball-api HTTP client with Keycloak auth (#11)

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

Subscribe

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

forgejo_admin/westside-ai-assistant!11

No description provided.