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

(FEAT) allow setting base url for openai v1.0.0+ #32

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ishaan-jaff
Copy link

this allows users to specify a custom openai api base

I tried export OPENAI_API_BASE but it looks like the latest version of openai does not read the base url from the .env

@@ -55,13 +55,14 @@ def prepare_openai_complete(is_async=False, api_key=None):

openai_obj = openai
is_v1 = False
base_url = os.environ.get("OPENAI_API_BASE", "https://api.openai.com/v1")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaults to https://api.openai.com/v1 if none specified

@ishaan-jaff
Copy link
Author

can i get a review on this @ankrgyl

@ankrgyl
Copy link
Contributor

ankrgyl commented Nov 14, 2023

If I'm researching correctly, OPENAI_API_BASE isn't a standard environment variable for the openai SDK. Instead, there's a base_url (baseURL in JS) parameter you can thread through. At a minimum, I'd prefer to support threading that through in a manner similar to api_url, so we can be consistent with the OpenAI SDK.

@ishaan-jaff
Copy link
Author

updated this to pass base_url as a param

Copy link
Contributor

@ankrgyl ankrgyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add in JS too?

@@ -35,7 +35,8 @@ def open_cache():
CACHE_LOCK = threading.Lock()


def prepare_openai_complete(is_async=False, api_key=None):
def prepare_openai_complete(is_async=False, api_key=None, base_url="https://api.openai.com/v1"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would rather not hardcode the API url here -- e.g. someone may monkey patch the library to override the default, and this would override that. Is there any issue with passing None into the OpenAI object by default?

@ankrgyl
Copy link
Contributor

ankrgyl commented Nov 28, 2023

@ishaan-jaff I think #33 supersedes this change (it allows you to optionally override the base url and supports JS).

Feel free to take a look, and happy to either rebase/scope this PR or just close it out.

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

Successfully merging this pull request may close these issues.

2 participants