Add Python 3.14 support#39
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 15 minutes and 27 seconds.Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds Python 3.14 support by modernizing typing usage, making event loop handling explicit and compatible with newer Python versions, and extending CI coverage to Python 3.14. Sequence diagram for application event loop initialization under Python 3.14sequenceDiagram
actor Developer
participant Application
participant Asyncio
Developer->>Application: create_application
activate Application
Application->>Asyncio: get_running_loop()
alt running loop exists
Asyncio-->>Application: returns existing_loop
Application->>Application: set loop = existing_loop
Application->>Application: set _sharing_loop = True
else no running loop
Asyncio-->>Application: raises RuntimeError
Application->>Asyncio: new_event_loop()
Asyncio-->>Application: returns new_loop
Application->>Application: set loop = new_loop
Application->>Application: set _sharing_loop = False
end
deactivate Application
Application-->>Developer: initialized application instance
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
1a3260f to
5289f30
Compare
5289f30 to
9120db2
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When creating a new event loop in
applications.pyonRuntimeError, consider setting it as the current loop and ensuring it is eventually closed to avoid leaking loops and to keep behavior closer to the previousget_event_loop()semantics. - In the
event_loopfixture, you create a new loop but never install it viaasyncio.set_event_loop(loop), which may surprise code that relies onget_event_loop()returning the same loop used by tests; consider explicitly setting and restoring the event loop in the fixture.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When creating a new event loop in `applications.py` on `RuntimeError`, consider setting it as the current loop and ensuring it is eventually closed to avoid leaking loops and to keep behavior closer to the previous `get_event_loop()` semantics.
- In the `event_loop` fixture, you create a new loop but never install it via `asyncio.set_event_loop(loop)`, which may surprise code that relies on `get_event_loop()` returning the same loop used by tests; consider explicitly setting and restoring the event loop in the fixture.
## Individual Comments
### Comment 1
<location path="natsapi/asyncapi/utils.py" line_range="64-65" />
<code_context>
"""
- if type(r) is typing._UnionGenericAlias:
- return list(r.__args__)
+ if typing.get_origin(r) is Union:
+ return list(typing.get_args(r))
else:
return [r]
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider using `typing.Union` explicitly or comparing to `types.UnionType` for better forward compatibility.
Using bare `Union` relies on it being imported and only matches `typing.Union`, not `|`-style unions (`types.UnionType`). For annotations using `|` (Python 3.10+), `typing.get_origin(r)` returns `types.UnionType`, so the `is Union` check will miss them. Consider something like:
```python
origin = typing.get_origin(r)
if origin in (typing.Union, types.UnionType):
return list(typing.get_args(r))
```
so both forms are handled.
Suggested implementation:
```python
:r Single or multiple response models
"""
origin = typing.get_origin(r)
if origin in (typing.Union, types.UnionType):
return list(typing.get_args(r))
else:
return [r]
```
1. At the top of `natsapi/asyncapi/utils.py`, ensure `types` is imported, e.g. add `import types` alongside the existing imports.
2. Ensure `typing` is imported as `import typing` (if not already) so `typing.Union`, `typing.get_origin`, and `typing.get_args` are available.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
9120db2 to
52569db
Compare
52569db to
fcf1332
Compare
Summary by Sourcery
Add support for running the project and its tests on Python 3.14 by modernizing type inspection and event loop handling.
New Features:
Bug Fixes:
CI:
Tests: