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

Adding support for non-Azure openai instances #507

Merged
merged 27 commits into from
Sep 14, 2023

Conversation

carlotta94c
Copy link
Contributor

Purpose

This PR adds OpenAI API support for users who don't have access to Azure OpenAI. Additions:

  • Calling OpenAI endpoint if env variable OPENAI_API_TYPE is set to openai (default models used: text-davinci-003, gpt-3.5-turbo and text-embedding-ada-002)
  • Making bicep configuration conditional (Azure OpenAI resource is created only if OPENAI_API_TYPE env variable is set to 'azure')
  • Adding instruction to switch from an Azure OpenAI endpoint (default) to a non-azure OpenAI instance

Does this introduce a breaking change?

[ ] Yes
[x ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code
    Configure an existing non-Azure OpenAI instance and deploy the app:
  1. Run azd env set OPENAI_API_TYPE openai
  2. Run azd env set OPENAI_ORGANIZATION {Your OpenAI organization}
  3. Run azd env set OPENAI_API_KEY {Your OpenAI API key}
  4. Run azd up
  5. Test the app by running it locally (Run Task -> Start App in VSCode) and in the Cloud (click on the link printed in the console after deployment)

What to Check

Verify that the following are valid

  • Running 'azd up' without changing env variables settings described above will ensure the same behavior of provisioning and deployment as before this PR (using Azure OpenAI resource is the default).

Other Information

@bgadrian
Copy link

Is there an extra config step is being missed out from the readme? openai API responds with an error "that the model is not specified", when the embeddings are done.

@carlotta94c
Copy link
Contributor Author

Is there an extra config step is being missed out from the readme? openai API responds with an error "that the model is not specified", when the embeddings are done.

@bgadrian thanks for the heads up! Yes I did miss the configuration of the openai embedding model name. I added it into the bicep file.

@pamelafox
Copy link
Collaborator

@carlotta94c I've merged main, made some style changes, verified that Azure OpenAI still works, and added unit tests (which resulted in snapshot updates due to the paramaterization). Please verify that non-Azure OpenAi deployment still works for you.

@carlotta94c
Copy link
Contributor Author

@carlotta94c I've merged main, made some style changes, verified that Azure OpenAI still works, and added unit tests (which resulted in snapshot updates due to the paramaterization). Please verify that non-Azure OpenAi deployment still works for you.

@pamelafox I've add some fixes and tested again deploy and run with non-Azure openai endpoint. It's working well now!

Copy link
Collaborator

@pamelafox pamelafox left a comment

Choose a reason for hiding this comment

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

Okay, there's still a non-optimal user experience for non-Azure OpenAI users where they get prompted for the location, but everything seems to work. Thanks for all the work on this Carlotta.

@pamelafox pamelafox merged commit 62e2248 into Azure-Samples:main Sep 14, 2023
6 checks passed
HughRunyan pushed a commit to RMI/RMI_chatbot that referenced this pull request Mar 26, 2024
* Adding openai non-azure endpoint support

* Editing infra files

* Adding missing args for the prepdocs script

* Rolling back indentation changes

* rolling back indentiation edits

* removing unnecessary parenthesis

* Adding OPENAI_EMB_MODEL parameter to bicep config

* Adding conditional args to reduce if statements

* Rm notebooks again

* OpenAI api type

* Tests pass

* Fix bicep issue

* Prepdocs fix

* Add main.bicep changes

* Add main.bicep changes

* Add missing api_type

* Parameterize tests

* Rm comma

* Making chatGptModelName param conditional

* Conditional system identity

* Make Azure_openai_service conditional

* fix inconsistencies across emb model param naming

* Fix formatting after merge from main

* Script fix

---------

Co-authored-by: Pamela Fox <pamela.fox@gmail.com>
Co-authored-by: Pamela Fox <pamelafox@microsoft.com>
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.

3 participants