-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add Custom SSL Certificate Verification in wattime/api.py #24
Comments
@Henry-WattTime @sam-watttime @xginn8 Hello, I hope you're doing well! I wanted to follow up on the Pull Request I submitted last week regarding adding support for custom SSL certificate verification; I've opened a new issue above and my team and I would really appreciate your feedback. Please let me know if there are any changes you'd like to see, or if there's anything preventing the PR from getting merged. Thanks! |
Hi @malakhalifa0 and thanks for this contribution! Since we're just using the Please let us know if you're able to test that and what the results are!! |
Thanks @xginn8 for the tip! It does work locally, but we believe it's good practice to have the verify parameter as an argument in the request, so that a certificate - if needed - could be passed explicitly. If we're using other python libraries, that under the hood are using REQUESTS_CA_BUNDLE, setting this env variable can change the performance of these or disrupt their behaviour. Having the verify parameter in Watttime's SDK would also significantly help with the debugging. |
Currently, Watttime does not support specifying a custom SSL certificate for verification when making requests. It defaults to using the system's SSL certificate store, which might not be suitable for all environments or security requirements.
To improve security and compatibility with specific use cases, watttime/api.py should be updated to allow users to specify a custom certificate path for SSL verification. This would ensure that API calls can trust a specific certificate rather than relying solely on the system's certificate store.
PR Reference: I've submitted a Pull Request that implements this feature (#23). Please review the changes and let me know if there are any adjustments needed.
The text was updated successfully, but these errors were encountered: