Skip to content
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

Allow for custom SSL certs to be used by the APIClient classes #1204

Closed
1 task done
tdene opened this issue Mar 1, 2024 · 6 comments · May be fixed by #1205
Closed
1 task done

Allow for custom SSL certs to be used by the APIClient classes #1204

tdene opened this issue Mar 1, 2024 · 6 comments · May be fixed by #1205
Labels
enhancement New feature or request

Comments

@tdene
Copy link

tdene commented Mar 1, 2024

Confirm this is an issue with the Python library and not an underlying OpenAI API

  • This is an issue with the Python library

Describe the bug

Currently, the APIClient classes allow for custom timeout, custom headers, custom max_retries, to be easily passed in as parameters.

The only way to pass in custom SSL certs are by one of these two options:

  • Setting the SSL_CERT_FILE environment variable. This causes issues with async calls.
  • Passing in a custom http_client. This causes inconsistencies, and is unnecessarily complicated when handling both Sync and Async clients through a 3rd party library like langchain.

Instead, verify should be an argument that can be passed in just like timeout or max_retries.

PR #1205 fixes this issue.

To Reproduce

  1. Try to use custom SSL certs.
  2. Note the difficulty in doing so.
  3. Merge in fix: make verify a client parameter (openai#1204) #1205 to make the task simple.

Code snippets

No response

OS

Ubuntu 20.04.5 LTS

Python version

Python v.3.11.6

Library version

openai v1.13.3

@tdene tdene added the bug Something isn't working label Mar 1, 2024
tdene added a commit to tdene/openai-python that referenced this issue Mar 1, 2024
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
@rattrayalex rattrayalex added enhancement New feature or request and removed bug Something isn't working labels Mar 2, 2024
@rattrayalex
Copy link
Collaborator

Setting the SSL_CERT_FILE environment variable. This causes issues with async calls.

How so? Can you share more about this?

@rattrayalex
Copy link
Collaborator

rattrayalex commented Mar 2, 2024

FWIW I think we are more likely to simply make it easier to configure a custom client, as discussed here: #1190 (comment)

@tdene
Copy link
Author

tdene commented Mar 6, 2024

Setting the SSL_CERT_FILE environment variable. This causes issues with async calls.

How so? Can you share more about this?

It has to do with connecting to multiple openai-compatible servers in the same piece of user code, while the various servers have different SSL certs. Continually setting and unsetting the env-var while running async code will lead to the wrong value being read at times. Wrapping everything in an asyncio lock ends up being more difficult than using the verify parameter.

Apologies, I don't have any shareable reproduction steps.

@rattrayalex
Copy link
Collaborator

rattrayalex commented Mar 6, 2024

Got it, that makes sense - thank you.

Do you think this would work just as well for you?

import openai

client = openai.OpenAI(
  http_client=openai.DefaultHttpxClient(verify=…),
)

@tdene
Copy link
Author

tdene commented Mar 7, 2024

That solution would work¹. When I originally posted this I had not noticed the other discussion thread, but I've been keeping up since then.

That's part of how I'm doing it already, and a DefaultHttpxClient would make for very nice syntactic sugar. Note that an async equivalent is also needed.

Feel free to close this issue and the associated PR as completed, and I will keep an eye out for DefaultHttpxClient and update my code then.

¹ The pain with the http_client parameter comes from the langchain side (they try to use the same http_client for both sync and async), not from yours. Apologies that I was pushing the problem onto you. Once you release the DefaultHttpxClient syntactic sugar, I'll make a PR on langchain to integrate it.

@rattrayalex
Copy link
Collaborator

Terrific, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants