-
Notifications
You must be signed in to change notification settings - Fork 186
feat: MTLS support for the InfluxDB Python client #509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @bednar, should I also prepare an integration test scenario, or is it for these optional parameters not necessary? |
Hi Jakob @bednar, could you please approve the missing workflows and review the change. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR 👍
There are a few requirements that must be be satisfy before we accept the PR:
Codecov ReportBase: 90.11% // Head: 90.14% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #509 +/- ##
==========================================
+ Coverage 90.11% 90.14% +0.02%
==========================================
Files 39 39
Lines 3380 3400 +20
==========================================
+ Hits 3046 3065 +19
- Misses 334 335 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your PR 👍 There are a few requirements that must be be satisfy before we accept the PR:
- Rebase your sources with
master
branch - Add new client's options (
:key str cert_file
, ...) to thefrom_config_file
andfrom_env_properties
functions. For more info see: feat: allow to use client's optional configs for initialization from file or environment properties #510
and also:
Hi @bednar, I'll adapt the PR. I'm also thinking about the possibility to add all SSL-related optional parameters like |
Yes, it will be useful. We already improve this docs in #510, please rebase your source to check an actual version of the client. |
BREAKING CHANGE: Rename `key_file` to `cert_key_file` inside the central configuration class Co-authored-by: Jakub Bednář <[email protected]>
I've handled it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are almost ready to merge this PR, but there are a few requirements that must be be satisfy before we accept the PR:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Closes #508
Proposed Changes
Add MTLS support to the InfluxDB Python client as already discussed inside the following issue.
TODO
Checklist
pytest tests
completes successfully