refactor: simplify WriteApi request path - #237
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
==========================================
- Coverage 86.90% 83.59% -3.31%
==========================================
Files 28 28
Lines 2084 2030 -54
==========================================
- Hits 1811 1697 -114
- Misses 273 333 +60 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal WriteApi write path to construct v2/v3 write requests directly and send them through the internal RestClient, removing leftover generated OpenAPI request plumbing while preserving existing write behaviors and updating tests to assert the RestClient boundary.
Changes:
- Replace generated-style request preparation (
call_api/_post_write_prepare/ serialization helpers) with_build_write_request(...)+_request(...)usingRestClient. - Update unit tests (including Polars) to validate request construction and mock
rest_client.requestinstead ofcall_api. - Update the changelog to mention the additional simplification of the
WriteApirequest path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
influxdb_client_3/write_client/client/write_api.py |
Removes generated OpenAPI request plumbing and routes write requests through _build_write_request + RestClient. |
tests/test_write_api.py |
Adds request-matrix coverage for _build_write_request and adjusts mocks to the RestClient boundary. |
tests/test_polars.py |
Updates Polars write tests to mock/assert rest_client.request calls and payload forwarding. |
CHANGELOG.md |
Notes further write-path simplification via direct RestClient request construction. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
influxdb_client_3/write_client/client/write_api.py:690
_build_write_requestraises errors that say "when calling_post_write", but the validation is happening inside_build_write_request. Using the correct method name makes errors clearer, especially when this helper is called directly in tests or frompost_write_async.
if org is None:
raise ValueError("Missing the required parameter `org` when calling `_post_write`")
if bucket is None:
raise ValueError("Missing the required parameter `bucket` when calling `_post_write`")
karel-rehor
left a comment
There was a problem hiding this comment.
A question regarding how this is documented in CHANGELOG.md needs to be clarified. Will continue with reviewing other changes.
| ### Breaking Changes | ||
|
|
||
| 1. [#217](https://github.com/InfluxCommunity/influxdb3-python/pull/217): Makes the writing API simpler and more consistent with other v3 clients. | ||
| 1. [#217](https://github.com/InfluxCommunity/influxdb3-python/pull/217), [#237](https://github.com/InfluxCommunity/influxdb3-python/pull/237): Makes the writing API simpler and more consistent with other v3 clients. |
There was a problem hiding this comment.
I see the changes in this PR are being documented as part of release 0.21.0, which was released last week. Are we planning to re-release 0.21.0? Or should this PR and its changes be added to the 0.22.0 release cycle?
This addendum to last week's release could lead users to understand that functionality has changed in 0.21.0, when in fact the change will only be available in 0.22.0.
There was a problem hiding this comment.
Fixed - updated 0.22.0 changelog.
Added bug fix details for version 0.22.0 and updated breaking changes section.
Further simplifies the WriteApi request path by constructing v2/v3 requests directly through RestClient, while preserving existing write behavior.
Summary
Testing
Closes #229