refactor: remove signin/signout and basic auth from write client#215
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the generated SigninService/SignoutService OpenAPI client surfaces and the associated session-cookie management logic from the write client, leaving only WriteService as the exposed service.
Changes:
- Delete
signin_service.pyandsignout_service.pyand stop re-exporting them from package/service/client__init__.pymodules. - Remove session-cookie lifecycle helpers from
write_client/rest.py. - Refactor
_sync/api_client.pyto drop cookie handling and sign-in/sign-out behavior (but currently leaves a broken import that must be fixed).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| influxdb_client_3/write_client/service/signout_service.py | Deleted the generated signout service API module. |
| influxdb_client_3/write_client/service/signin_service.py | Deleted the generated signin service API module. |
| influxdb_client_3/write_client/service/init.py | Stops exporting SigninService/SignoutService; exports WriteService only. |
| influxdb_client_3/write_client/rest.py | Removes user-session helper functions related to signin/signout cookie handling. |
| influxdb_client_3/write_client/client/write/init.py | Stops re-exporting SigninService/SignoutService from the write client package. |
| influxdb_client_3/write_client/client/init.py | Stops re-exporting SigninService/SignoutService from the client package. |
| influxdb_client_3/write_client/_sync/api_client.py | Removes cookie parameter/storage and sign-in/sign-out behavior, but currently retains an import of removed helpers. |
| influxdb_client_3/write_client/init.py | Stops re-exporting SigninService/SignoutService at the package level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
==========================================
+ Coverage 77.65% 78.90% +1.24%
==========================================
Files 38 36 -2
Lines 2721 2607 -114
==========================================
- Hits 2113 2057 -56
+ Misses 608 550 -58 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
influxdb_client_3/write_client/client/_base.py:72
auth_basic/username/passwordsupport appears to have been removed, but passing these kwargs will now be silently ignored, which can lead to confusing auth failures for existing consumers (especially sinceInfluxDBClient3forwards**kwargsinto the write client). Consider explicitly rejecting these removed options (or emitting a clear deprecation/error) so users fail fast with actionable guidance.
# defaults
self.auth_header_name = None
self.auth_header_value = None
# by token
if token:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
influxdb_client_3/write_client/client/_base.py:72
- Removed auth modes (auth_basic and username/password) are now silently ignored if a caller still passes them via **kwargs, which can lead to confusing, unauthenticated requests. Since this PR removes support, it would be safer to fail fast with a clear ValueError when these kwargs are provided (non-empty/true).
# defaults
self.auth_header_name = None
self.auth_header_value = None
# by token
if token:
7e58ae6 to
b55fb6a
Compare
|
LGTM except for the title 😃 |
My bad... forgot to change it 😹 |
|
may I suggest something more descriptive, such as
|
Done |
Closes #214
Proposed Changes
Checklist