feat(sdk/python): add python baseline sdk#4711
Conversation
mark-vass-konghq
added
area/api
release-note/feature
📝 WalkthroughWalkthroughThis PR adds an OpenMeter Python SDK v3 baseline with typed models, synchronous and asynchronous clients, HTTP transports with retries and bounded responses, meter and plan add-on resources, CSV support, packaging metadata, examples, documentation, and integration tests. ChangesPython SDK v3
Estimated code review effort: 5 (Critical) | ~90 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Application
participant Client
participant Resource
participant Transport
participant OpenMeterAPI
Application->>Client: create SDK client
Application->>Resource: invoke meter or plan add-on operation
Resource->>Transport: build and send request
Transport->>OpenMeterAPI: HTTP request
OpenMeterAPI-->>Transport: response or problem details
Transport-->>Resource: parsed model, bytes, stream, or error
Resource-->>Application: operation result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return self._transport.request_json( | ||
| "PUT", | ||
| _resource_path(_METERS_PATH, meter_id), | ||
| Meter, | ||
| body=request, | ||
| ) |
There was a problem hiding this comment.
When callers pass UpdateMeterRequest(labels=None) or UpdateMeterRequest(dimensions=None), this path uses the shared serializer that drops None fields before sending the PUT body. That makes the documented clear operation indistinguishable from an omitted field, so a server that preserves absent fields can return success while existing labels or dimensions remain unchanged.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/python/openmeter/src/openmeter/client.py
Line: 112-117
Comment:
**Clear Fields Are Omitted**
When callers pass `UpdateMeterRequest(labels=None)` or `UpdateMeterRequest(dimensions=None)`, this path uses the shared serializer that drops `None` fields before sending the PUT body. That makes the documented clear operation indistinguishable from an omitted field, so a server that preserves absent fields can return success while existing labels or dimensions remain unchanged.
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
sdk/python/openmeter/src/openmeter/models.py (1)
155-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "exactly one operator" validator logic.
_StringQueryFilter.validate_operator(Lines 175-183) re-implements the exact same "count non-None fields, require exactly 1" pattern already written inStringFilter.validate_operator(Lines 119-131). Worth extracting a small shared helper (e.g._require_single_operator(model)) so the two validators can't silently drift apart.♻️ Suggested extraction
+def _require_single_operator(model: BaseModel, error: str) -> None: + values = [getattr(model, name) for name in model.__class__.model_fields] + if sum(value is not None for value in values) != 1: + raise ValueError(error) + + class StringFilter(_RequestModel): ... `@model_validator`(mode="after") def validate_operator(self) -> StringFilter: - values = [getattr(self, name) for name in self.__class__.model_fields] - if sum(value is not None for value in values) != 1: - raise ValueError("exactly one string filter operator must be set") + _require_single_operator(self, "exactly one string filter operator must be set") ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/python/openmeter/src/openmeter/models.py` around lines 155 - 183, Extract the shared “exactly one non-None operator” check from StringFilter.validate_operator and _StringQueryFilter.validate_operator into a small helper such as _require_single_operator(model). Update both validators to call that helper while preserving their existing validation error behavior and return semantics.sdk/python/openmeter/src/openmeter/_transport.py (1)
21-26: 🩺 Stability & Availability | 🔵 TrivialBlanket POST-exclusion from retries also blocks retries for read-only query calls.
The rationale here (avoid double-creating resources) is sound for
meters.create/plan_addons.create, butmeters.query/query_csv/query_csv_streamalso use POST purely to carry a filter body — they're read-only and safe to retry on a transient 502/503/504, yet get zero automatic retries under the current method-based classification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/python/openmeter/src/openmeter/_transport.py` around lines 21 - 26, Update the retry classification around _RETRYABLE_METHODS so read-only meter query operations (query, query_csv, and query_csv_stream) can retry transient 502/503/504 responses despite using POST, while preserving no-retry behavior for resource-creating POST calls such as meters.create and plan_addons.create. Use the existing transport request context or operation identity to distinguish safe query calls from mutating POST requests.sdk/python/openmeter/src/openmeter/errors.py (1)
56-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider exposing
invalid_parametersas a convenience property too.
title/detail/type/instanceall get a friendly property, butinvalid_parameters(a field callers will often want when reporting which request field failed validation) only reachable viaerror.problem.invalid_parameters. Small addition for symmetry:`@property` def invalid_parameters(self) -> list[InvalidParameter]: """Per-field validation issues reported by the API, if any.""" return self.problem.invalid_parameters🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/python/openmeter/src/openmeter/errors.py` around lines 56 - 78, Expose invalid_parameters on the same error class as the existing title, detail, type, and instance properties by adding a typed convenience property that returns self.problem.invalid_parameters and documents the per-field validation issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/python/openmeter/pyproject.toml`:
- Around line 46-48: Update the setuptools configuration near
[tool.setuptools.packages.find] to explicitly include the py.typed marker in the
openmeter package data, ensuring it is bundled into the wheel for downstream
type checkers.
In `@sdk/python/openmeter/src/openmeter/_transport_async.py`:
- Around line 150-159: Update the async transport request handling around the
status-code check in `_transport_async.py` to configure the underlying client
not to follow redirects, matching `_Transport.__init__` in `_transport.py`.
Preserve the existing 2xx success handling and error processing while ensuring
3xx responses are surfaced consistently with the sync transport.
In `@sdk/python/openmeter/src/openmeter/_transport.py`:
- Around line 31-51: The _Transport initializer currently uses build_opener()
with urllib’s default redirect handling, unlike AsyncClient. Configure the
opener in _Transport.__init__ to disable automatic HTTP redirects so 3xx
responses are surfaced and handled consistently as APIError; add a sync
transport test verifying redirects are not followed.
In `@sdk/python/openmeter/src/openmeter/aio.py`:
- Around line 248-261: Update the query_csv_stream docstring in the async client
to state that the caller must close the returned AsyncByteStream, including
guidance to use its close or async context-manager support. Keep the method’s
streaming behavior unchanged.
---
Nitpick comments:
In `@sdk/python/openmeter/src/openmeter/_transport.py`:
- Around line 21-26: Update the retry classification around _RETRYABLE_METHODS
so read-only meter query operations (query, query_csv, and query_csv_stream) can
retry transient 502/503/504 responses despite using POST, while preserving
no-retry behavior for resource-creating POST calls such as meters.create and
plan_addons.create. Use the existing transport request context or operation
identity to distinguish safe query calls from mutating POST requests.
In `@sdk/python/openmeter/src/openmeter/errors.py`:
- Around line 56-78: Expose invalid_parameters on the same error class as the
existing title, detail, type, and instance properties by adding a typed
convenience property that returns self.problem.invalid_parameters and documents
the per-field validation issues.
In `@sdk/python/openmeter/src/openmeter/models.py`:
- Around line 155-183: Extract the shared “exactly one non-None operator” check
from StringFilter.validate_operator and _StringQueryFilter.validate_operator
into a small helper such as _require_single_operator(model). Update both
validators to call that helper while preserving their existing validation error
behavior and return semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc2242be-ca13-400b-9e86-862179535937
⛔ Files ignored due to path filters (1)
sdk/python/openmeter/uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (18)
sdk/python/openmeter/.flake8sdk/python/openmeter/.gitignoresdk/python/openmeter/MANIFEST.insdk/python/openmeter/README.mdsdk/python/openmeter/examples/async.pysdk/python/openmeter/examples/sync.pysdk/python/openmeter/pyproject.tomlsdk/python/openmeter/src/openmeter/__init__.pysdk/python/openmeter/src/openmeter/_transport.pysdk/python/openmeter/src/openmeter/_transport_async.pysdk/python/openmeter/src/openmeter/_version.pysdk/python/openmeter/src/openmeter/aio.pysdk/python/openmeter/src/openmeter/client.pysdk/python/openmeter/src/openmeter/errors.pysdk/python/openmeter/src/openmeter/models.pysdk/python/openmeter/src/openmeter/py.typedsdk/python/openmeter/tests/__init__.pysdk/python/openmeter/tests/test_sdk.py
| [tool.setuptools.packages.find] | ||
| where = ["src"] | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Ensure the py.typed marker gets bundled in the wheel.
Hey! I see we're building a typed SDK (love the Typing :: Typed classifier!), but we need to make sure the py.typed marker file actually makes it into the installed wheel. setuptools won't grab non-Python files automatically unless we explicitly specify them in package-data.
Let's add that so downstream type checkers know the package is fully typed!
📦 Proposed fix to include the marker file
[tool.setuptools.packages.find]
where = ["src"]
+
+[tool.setuptools.package-data]
+openmeter = ["py.typed"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tool.setuptools.packages.find] | |
| where = ["src"] | |
| [tool.setuptools.packages.find] | |
| where = ["src"] | |
| [tool.setuptools.package-data] | |
| openmeter = ["py.typed"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/python/openmeter/pyproject.toml` around lines 46 - 48, Update the
setuptools configuration near [tool.setuptools.packages.find] to explicitly
include the py.typed marker in the openmeter package data, ensuring it is
bundled into the wheel for downstream type checkers.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
sdk/python/openmeter/pyproject.toml (1)
51-53: 📐 Maintainability & Code Quality | 🟠 MajorInclude
py.typedin the wheel.This configuration only discovers Python packages. Unless another build-data mechanism covers it,
src/openmeter/py.typedmay be omitted from the wheel, causing downstream type checkers to miss the typed-package marker. Add explicit package data and verify the built wheel contents.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/python/openmeter/pyproject.toml` around lines 51 - 53, Update the setuptools configuration in pyproject.toml to explicitly include the src/openmeter/py.typed marker as package data, ensuring it is present in the built wheel. Verify the wheel archive contains openmeter/py.typed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@sdk/python/openmeter/pyproject.toml`:
- Around line 51-53: Update the setuptools configuration in pyproject.toml to
explicitly include the src/openmeter/py.typed marker as package data, ensuring
it is present in the built wheel. Verify the wheel archive contains
openmeter/py.typed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd0198c2-d720-414b-b565-f95d9d73a383
⛔ Files ignored due to path filters (1)
sdk/python/openmeter/uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (8)
sdk/python/openmeter/README.mdsdk/python/openmeter/pyproject.tomlsdk/python/openmeter/src/openmeter/__init__.pysdk/python/openmeter/src/openmeter/_transport.pysdk/python/openmeter/src/openmeter/_transport_async.pysdk/python/openmeter/src/openmeter/aio.pysdk/python/openmeter/src/openmeter/errors.pysdk/python/openmeter/tests/test_sdk.py
🚧 Files skipped from review as they are similar to previous changes (5)
- sdk/python/openmeter/src/openmeter/errors.py
- sdk/python/openmeter/README.md
- sdk/python/openmeter/src/openmeter/init.py
- sdk/python/openmeter/src/openmeter/aio.py
- sdk/python/openmeter/src/openmeter/_transport_async.py
1 participant
Summary
Adds an idiomatic, typed Python SDK baseline for the OpenMeter v3 API, covering meters and plan add-ons with matching synchronous and asynchronous clients.
What changed
This is intentionally a baseline SDK rather than complete v3 API coverage.
Verification
uv run --frozen flake8 src tests examplesuv run --frozen ruff check .uv run --frozen ruff format --check .uv run --frozen pyrightuv run --frozen python -m unittest discover -s tests -v— 28 tests passeduv buildSummary by CodeRabbit
Greptile Summary
This PR adds a baseline Python SDK for the OpenMeter v3 API. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/python-sdk..." | Re-trigger Greptile
Context used (3)