fix(client-python): import NotRequired from typing_extensions for 3.10 support - #2040
Conversation
…0 support
The package declares requires-python >=3.10 and ships the 3.10 trove
classifier, but types/billing.py and types/client.py both import
NotRequired directly from typing, which only gained it in Python 3.11
(PEP 655). On 3.10, `import rocketride` fails outright:
ImportError: cannot import name 'NotRequired' from 'typing'
client.py is imported before billing.py in types/__init__.py, so the
failure surfaces there first - fixing billing.py alone would not have
resolved the reported error.
Import NotRequired from typing_extensions in both files instead, and
add typing_extensions as an explicit dependency (it was previously
only present transitively via pydantic, so not something this package
declared or controlled).
Verified on real interpreters, not just by reading the code:
python3.10 -c "...; import rocketride" # ImportError before, OK after
python3.11 -c "...; import rocketride" # OK before and after
Ran the full client-python test suite before and after: 48 failed / 52
passed / 6 skipped / 21 errors in both cases (the failures are
pre-existing integration tests that need a live RocketRide engine
connection - unrelated to this change, confirmed identical on a clean
checkout).
Fixes rocketride-org#1820
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe Python client adds ChangesPython typing compatibility
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change makes the Python client importable on Python 3.10 by using the supported compatibility package and declaring it directly; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. Comment |
Description
Fixes #1820.
packages/client-python/pyproject.tomldeclaresrequires-python = ">=3.10"and ships theProgramming Language :: Python :: 3.10trove classifier, buttypes/billing.pyandtypes/client.pyboth importNotRequireddirectly fromtyping, which only gained it in Python 3.11 (PEP 655). On 3.10,import rocketridefails outright.One correction to the issue as filed: it identifies
billing.pyas the sole offender ("Scope is exactly one file... billing.py holds all 17 NotRequired uses").types/client.pyhas the same unconditionalfrom typing import ..., NotRequired, ...(line 58, used at line 227 fordeveloperId). Sincetypes/__init__.pyimportsclientbeforebilling, the reported traceback actually surfaces inclient.pyfirst — fixingbilling.pyalone would not have resolved the reported error. Both files are fixed here.Fix
from typing import ..., NotRequired, ...→ split into atypingimport (unaffected names) plusfrom typing_extensions import NotRequired, in both files.typing_extensions>=4.0.0todependenciesinpyproject.toml— it was previously only present transitively viapydantic, not something this package declared or controlled (matches the issue's own note on this).Testing performed
Reproduced and verified against real interpreters, not just by reading the code:
Also ran the full
client-pythontest suite before and after this change:Identical counts on both a clean
developcheckout and this branch — the failures are pre-existing integration tests that require a live RocketRide engine connection, unrelated to this change.Breaking changes
None. Adds one new direct dependency (
typing_extensions) that was already present transitively viapydantic, so no new resolution constraint in practice.Fixes #1820
Summary by CodeRabbit