refactor: multiprocessing helper - #222
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
==========================================
- Coverage 87.21% 86.88% -0.33%
==========================================
Files 28 28
Lines 2072 2082 +10
==========================================
+ Hits 1807 1809 +2
- Misses 265 273 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
6fbdf4b to
f227120
Compare
706407e to
53ac912
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the write stack by removing the generated OpenAPI-style service/client layer and moving write request construction and HTTP dispatch directly into WriteApi, while also updating tests and documentation to reflect the new behavior and configuration surface.
Changes:
- Refactors
WriteApito perform request preparation + HTTP calls directly via a new low-levelRestClient(removingApiClient/WriteService/Configuration). - Updates
InfluxDBClient3initialization and tests to use the new header/rest client approach and to validate required constructor keys. - Refactors and adds integration coverage for
MultiprocessingWriterstart-method behavior and directWriteApiusage.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents breaking changes and new construction patterns (example needs a small syntax fix). |
| influxdb_client_3/init.py | Reworks InfluxDBClient3 construction to create RestClient + WriteApi directly and enforce required keys. |
| influxdb_client_3/write_client/init.py | Removes deprecated exports and re-exports updated WriteApi/types. |
| influxdb_client_3/write_client/_sync/api_client.py | Removes legacy generated ApiClient. |
| influxdb_client_3/write_client/_sync/rest.py | Removes legacy generated REST client implementation. |
| influxdb_client_3/write_client/_sync/rest_client.py | Adds new RestClient for low-level HTTP handling + logging/redaction. |
| influxdb_client_3/write_client/client/init.py | Removes re-export of removed WriteService. |
| influxdb_client_3/write_client/client/_base.py | Removes legacy base client/write-api helpers tied to generated stack. |
| influxdb_client_3/write_client/client/influxdb_client.py | Removes legacy v2 InfluxDBClient. |
| influxdb_client_3/write_client/client/logging_handler.py | Removes InfluxLoggingHandler implementation. |
| influxdb_client_3/write_client/client/util/helpers.py | Updates exception import path. |
| influxdb_client_3/write_client/client/util/multiprocessing_helper.py | Refactors MultiprocessingWriter to create WriteApi directly and set default start method. |
| influxdb_client_3/write_client/client/write/init.py | Removes legacy re-export of removed WriteService. |
| influxdb_client_3/write_client/client/write_api.py | Major refactor: WriteApi becomes standalone, builds write requests and calls RestClient directly, retains batching pipeline. |
| influxdb_client_3/write_client/configuration.py | Removes legacy generated configuration object. |
| influxdb_client_3/write_client/service/init.py | Removes legacy service package init exporting WriteService. |
| influxdb_client_3/write_client/service/_base_service.py | Removes legacy service base class. |
| influxdb_client_3/write_client/service/write_service.py | Removes legacy generated WriteService. |
| influxdb_client_3/write_client/write_exceptions.py | Removes _BaseRESTClient logger helper (moved to RestClient). |
| tests/test_influxdb_client_3.py | Updates unit tests for new client/write header/rest behavior and required params. |
| tests/test_influxdb_client_3_integration.py | Adds integration tests for direct WriteApi usage, multiprocessing helper, and async call path. |
| tests/test_polars.py | Updates write tests to mock WriteApi call sites instead of WriteService. |
| tests/test_query.py | Fixes typo in client init parameter (database). |
| tests/test_write_api.py | Refactors tests from generated service/client usage to InfluxDBClient3/WriteApi usage. |
| tests/test_write_local_server.py | Updates exception import path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f6ef07c to
ab75516
Compare
|
@bednar @karel-rehor @alespour |
24b67a3 to
aed0973
Compare
karel-rehor
left a comment
There was a problem hiding this comment.
This looks good so far.
I especially like the use of get_context() for each instance. It's cleaner, more flexible and more stable than what I suggested last week.
A couple of notes.
-
need to update CHANGELOG.md
-
The codecov report shows that important parts of the
runandterminatemethods are not covered by tests.
I think this is partially a false report. I suspect codecov is not tracking code executed in newly spawned processes. When running tests in the debugger, I see the processes enter this area of code. It's probably necessary to configure codecov to follow multiprocess executions.
e.g. in pyproject.toml (+AI+ suggestion 1)
[tool.coverage.run]
concurrency = ["multiprocessing"]
parallel = true
source = ["your_package_name"]or perhaps in .coveragerc (+AI+ suggestion 2)
[run]
concurrency = multiprocessing
parallel = true
source = your_package_nameIt was suggested in a PR for influxdb-client-php that CircleCI has a codecov orb that simplifies working with codecoverage reports. It might help resolve this issue.
e334ed5 to
4ea2e36
Compare
|
Hi @karel-rehor CHANGES:
TODO:
Please click "Resolve conversation" after all issues have been fixed. |
karel-rehor
left a comment
There was a problem hiding this comment.
I still have a couple of minor requests, regarding tests and CHANGELOG.md.
karel-rehor
left a comment
There was a problem hiding this comment.
Thanks for the last minute changes.
Looks good to me. 🚴 🏁
e718ed7 to
011c1b6
Compare
Closes #
Proposed Changes
MultiprocessingWriterclass:- WriteApi will be created and used directly in the class for writing.
- Tests are added for writing with multiprocessing.
- Use
DefaultContext.Process(target)to create a new process.- Users can now choose one of the start methods
fork,spawnorforkserverwhen creating a new Process. The default will bespawn.- Implement a time-to live mechanism for child processes. I just set timeout for JoinableQueue.get() so It will wait for a number of times, default is 300 seconds, and users can pass in their own value to override that by
process_ttlparameter.- Users can pass in an on_shutdown() function which gets called when the queue has been shutdown.
Checklist