Fiddle: small fix for streaming json route#132
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an inconsistency in streaming_json_route handler invocation (aligning it with json_route) and adds tests to validate bearer-JWT authorization behavior for both standard JSON and streaming NDJSON routes.
Changes:
- Update
streaming_json_routeto call wrapped handlers withrequest=(keyword argument). - Add end-to-end tests covering
authorize_bearer_jwtbehavior with bothjson_routeandstreaming_json_route.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/api/streaming_json_route.py | Aligns handler invocation style with json_route by passing the KibaApiRequest via request=. |
| tests/api/test_authorizer.py | Adds integration-style tests verifying 403/200 behavior and authJwt propagation for JSON and streaming routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| kibaRequest: KibaApiRequest[ApiRequest] = KibaApiRequest(scope=receivedRequest.scope, receive=receivedRequest._receive, send=receivedRequest._send) # noqa: SLF001 | ||
| kibaRequest.data = requestParams | ||
| responseGeneratorOrAwaitable = func(kibaRequest) | ||
| responseGeneratorOrAwaitable = func(request=kibaRequest) |
There was a problem hiding this comment.
streaming_json_route now calls the wrapped handler using a keyword argument (func(request=...)), which will raise a TypeError at runtime for handlers that don’t name their parameter request (even if they accept a single positional request). To make this change safer/clearer, either (a) update the decorator’s type hints to require an argument named request (similar to json_route using mypy_extensions.Arg), and/or (b) add a backward-compatible call path for positional-only handlers and document the expected handler signature.
| responseGeneratorOrAwaitable = func(request=kibaRequest) | |
| try: | |
| responseGeneratorOrAwaitable = func(request=kibaRequest) | |
| except TypeError: | |
| responseGeneratorOrAwaitable = func(kibaRequest) |
| import json | ||
| lines = [l for l in response.content.decode().strip().split('\n') if l] | ||
| assert len(lines) == 1 | ||
| data = json.loads(lines[0]) | ||
| assert data['result'] == 'hello' |
There was a problem hiding this comment.
The streaming tests re-implement NDJSON parsing inline and import json inside the test bodies. To keep test helpers consistent and reduce duplication, consider moving the json import to module scope and reusing/centralizing the NDJSON parsing approach already used in tests/api/test_streaming_json_route.py (see parse_ndjson_response), so future format/header changes only need to be updated in one place.
Description
Screenshots:
Checklist: