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

Feature: Switch OPENAI's API URL #49

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

jcrodriguez1989
Copy link
Owner

As requested in #48 , we need to be able to allow switching OPENAI's API URL.
This is now possible by setting the OPENAI_API_URL env variable.

Copy link

github-actions bot commented Nov 9, 2023

Thank you for sharing the GitHub pull request file. I will perform a code review to identify any issues.

In the code snippet provided, I noticed the following issue:

In the file "R/gpt_get_completions.R" on line 65, the URL for the POST request is hardcoded as "https://api.openai.com/v1/chat/completions". It is recommended to use the api_url variable instead to allow flexibility in changing the API URL. This can be fixed by replacing the hardcoded URL with paste0(api_url, "/chat/completions").

That is the only issue I found in the code snippet you shared.

Copy link

github-actions bot commented Nov 9, 2023

Code review findings:

  1. In R/chatgpt-package.R, line 14, it seems that api_url is being initialized in a non-standard way. It is recommended to use the ifelse function to check if the environment variable is set, and then assign the default value if it is not.

  2. In R/gpt_get_completions.R, line 65, where the API endpoint URL is constructed, it is good practice to include a trailing slash (/) after the base URL. This ensures consistency in the URL format and prevents unexpected behavior.

  3. In README.Rmd and README.md, the example usage of setting the OPENAI_API_URL environment variable should be updated to use the correct URL format. The current example (https://api.chatanywhere.com.cn) appears to be fictional.

Please make the necessary changes based on the above recommendations.

@jcrodriguez1989 jcrodriguez1989 merged commit c38f4cb into develop Nov 9, 2023
1 check passed
@jcrodriguez1989 jcrodriguez1989 deleted the feature.jcr.switch_api_url branch November 9, 2023 10:18
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.

1 participant