Add API request IDs, structured errors, pagination and filters#73
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22fff0accf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| token = credentials.credentials if credentials and credentials.scheme.lower() == "bearer" else None | ||
| # Preserve the legacy parser's exact "Bearer " prefix behavior for unusual clients. | ||
| if token is None and authorization and authorization.startswith("Bearer "): | ||
| token = authorization[len("Bearer "):].strip() | ||
| if token != settings.api_token: |
There was a problem hiding this comment.
When api_token is unset and allow_unauthenticated_dev is False, this logic still allows requests without an Authorization header: token stays None, and token != settings.api_token is False when both are None. That means protected endpoints become publicly accessible under a strict-auth configuration with a missing token, which is a security regression from the previous behavior.
Useful? React with 👍 / 👎.
| rules = stores.store.get_all_rules() | ||
| return {"rules": rules, "count": len(rules)} | ||
| filtered = [r for r in rules if (dataset is None or r.get("dataset") == dataset) and (enabled is None or r.get("enabled") == enabled) and (severity is None or r.get("severity") == severity) and (check_type is None or r.get("check_type", r.get("type")) == check_type)] | ||
| page = _paginate(filtered, limit, offset) | ||
| return {"rules": page["items"], "count": len(page["items"]), "pagination": page["pagination"]} |
There was a problem hiding this comment.
This endpoint now calls get_all_rules() without pagination arguments, then paginates the returned subset in memory. Because store defaults are limit=100, offset=0, requests like ?limit=500 or ?offset=120 can never access records beyond the first 100, and pagination.total is computed from that truncated slice rather than the real result set.
Useful? React with 👍 / 👎.
| def list_quality_results(self, limit: int = 100, offset: int = 0, table: Optional[str] = None, status: Optional[str] = None, dataset: Optional[str] = None, check_type: Optional[str] = None, severity: Optional[str] = None, run_id: Optional[str] = None) -> List[Dict[str, Any]]: | ||
| results = list(self._quality.values()) | ||
| if table: | ||
| results = [r for r in results if r.get("table") == table] | ||
| if status: |
There was a problem hiding this comment.
The in-memory implementation advertises check_type, severity, and run_id filters in its method signature, but it only applies table, status, and dataset. In memory-backed deployments/tests, those documented query params are silently ignored, producing incorrect results and backend-inconsistent behavior.
Useful? React with 👍 / 👎.
1 participant
Motivation
Description
X-Request-IDor generates a UUID, stores it onrequest.state, and echoes it on every response; request IDs are included in all structured error payloads (src/api/app.py).ErrorResponse(error.code,error.message,error.details,request_id) and wired handlers for generalHTTPException,StarletteHTTPException,RequestValidationError(400/422) and unhandled exceptions (500) (src/api/app.py).PaginationMeta) and appliedlimit/offsetquery parameters (defaults and caps viaQuery) toGET /rules,GET /quality/results,GET /lineage/nodes, andGET /lineage/edges, preserving the existing top-level keys (rules,results,nodes,edges) and addingpaginationmetadata (src/api/app.py).Queryparameters (src/api/app.py).limit/offsetand filter parameters, and updatedInMemoryStoreandElasticsearchStoreto perform filtering/pagination (pushed into ES where practical) while keeping backwards-compatible defaults (src/api/store.py,src/api/es_store.py).request_id, pagination and filter behaviour,limitmax enforcement and negativeoffsetvalidation; documentation added atdocs/api/rest-api.mdwith examples and curl snippets.Testing
python -m compileall -q src testsand it completed successfully (no compile failures).pytest tests/test_api_endpoints.py -q,pytest tests/test_api_es_store.py -q,pytest tests/test_settings.py -q,pytest tests/test_poc_pipeline.py -qand all targeted test files passed.pytest -qwhich completed successfully with all tests passing (suite result: tests passed; some integration tests remain skipped by default).Codex Task