You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add support to io/api to allow setting the SSL certificate for the requests HTTP GET/POST operations.
In the situation where solarforecastarbiter must be run from behind a HTTP proxy (e.g. due to corporate firewall/network restrictions), it can be necessary to provide a SSL certificate to requests operations (e.g. requests.get(<params>, vertify="some_certificate.crt") instead of requests.get(<params>)). I'm not sure about the exact implementation, but this should just require revisions to the io/api to allow a user to call the functions with an optional specified SSL certificate filepath (e.g. api.request_cli_access_token(username, password, sslcert="some_certificate.crt")).
Note: I did get this working as a proof-of-concept by hacking around in io/api and adding verify="some_certificate.cr" to all the requests.get() and requests.post() calls. Not ideal, but it works.
@dplarson thanks for the report. Your solution seems reasonable. I wonder if we'll need to support more keyword arguments for other network issues, in which case maybe passing generic **kwargs could be a way forward.
Thanks for the feedback. I'll confirm if REQUESTS_CA_BUNDLE works as expected. If it does, then I think it'll be sufficient to simply provide a note on using SSL certificates to io/api (e.g. a Notes section in the docstrings or maybe a page on the main website with "Common questions and issues").
But before closing this issue: would it beneficial to add a note to the io/api docstrings about using REQUESTS_CA_BUNDLE? Or do you think the existence of this issue is sufficient given that this is (likely) an issue affecting a small number of users (with myself being the only confirmed case so far)?
The overlap of people using the python API to get/post data and corporate firewalls is probably pretty small, but I expect it to come up again when it's in wider use. It wouldn't surprise me if users at national labs hit this a few times. So I do think it would be beneficial to add something to the docs.
Maybe add info to the APISession docstring here and/or the SFA API section of api.rst.
I think it's preferable if we can get by with relying on the established requests patterns of letting users set environment variables and/or attributes of APISession, rather than adding **kwargs to every method.
I also agree, with the one exception of io.api.request_cli_access_token. I think it is worth adding the **kwargs there since it uses a bare request vs a session. The env variable would still work, but good to have an option beyond that
Activity
wholmgren commentedon Jun 3, 2020
@dplarson thanks for the report. Your solution seems reasonable. I wonder if we'll need to support more keyword arguments for other network issues, in which case maybe passing generic
**kwargscould be a way forward.It looks like in your case an alternative is to set the
REQUESTS_CA_BUNDLEenvironment variable: https://requests.readthedocs.io/en/master/user/advanced/#ssl-cert-verificationalorenzo175 commentedon Jun 3, 2020
Yeah, maybe
**kwargsto all the request methods with appropriate documentation.Should also update the
APISessiondocumentation since it is a requests.Session and theAPISession.verify = '/path/to/cert'should work as well.https://requests.readthedocs.io/en/master/api/#sessionapi
dplarson commentedon Jun 3, 2020
Thanks for the feedback. I'll confirm if
REQUESTS_CA_BUNDLEworks as expected. If it does, then I think it'll be sufficient to simply provide a note on using SSL certificates toio/api(e.g. aNotessection in the docstrings or maybe a page on the main website with "Common questions and issues").dplarson commentedon Jun 4, 2020
I've confirmed that defining the
REQUESTS_CA_BUNDLEenv var (with no changes to therequestscalls inio/api) is sufficient for solving my issue:But before closing this issue: would it beneficial to add a note to the
io/apidocstrings about usingREQUESTS_CA_BUNDLE? Or do you think the existence of this issue is sufficient given that this is (likely) an issue affecting a small number of users (with myself being the only confirmed case so far)?wholmgren commentedon Jun 4, 2020
The overlap of people using the python API to get/post data and corporate firewalls is probably pretty small, but I expect it to come up again when it's in wider use. It wouldn't surprise me if users at national labs hit this a few times. So I do think it would be beneficial to add something to the docs.
Maybe add info to the
APISessiondocstring here and/or the SFA API section ofapi.rst.I think it's preferable if we can get by with relying on the established
requestspatterns of letting users set environment variables and/or attributes ofAPISession, rather than adding**kwargsto every method.dplarson commentedon Jun 4, 2020
Agreed on documentation being preferable to adding
**kwargsto each method. I'll open a PR to add the documentation changes.alorenzo175 commentedon Jun 4, 2020
I also agree, with the one exception of
io.api.request_cli_access_token. I think it is worth adding the**kwargsthere since it uses a bare request vs a session. The env variable would still work, but good to have an option beyond that