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

fix: OpenAIEmbeddings support for Azure OpenAI Service custom endpoints #3711

Conversation

zioproto
Copy link
Contributor

Related issue #1560

@zioproto
Copy link
Contributor Author

According to the docs:
https://learn.microsoft.com/en-us/azure/cognitive-services/openai/reference#embeddings

The POST request should look like:

POST https://{your-resource-name}.openai.azure.com/openai/deployments/{deployment-id}/embeddings?api-version={api-version}

But I am setting api_base to xxxxxx.openai.azure.com/openai and I am getting:

https://xxxxxx.openai.azure.com/openai/engines/text-embedding-ada-002/embeddings

I am not sure where engines comes from

@zioproto zioproto force-pushed the fix/api-base-azure-openai-embeddings branch from 067f6a5 to 8179f72 Compare April 28, 2023 14:47
@zioproto
Copy link
Contributor Author

I refactored the commit into 8179f728d3073765573a9c4ff07bb3ff717cb372

Now it is working for me as expected.

@zioproto zioproto force-pushed the fix/api-base-azure-openai-embeddings branch from 8179f72 to 279a59b Compare April 29, 2023 06:16
@zioproto
Copy link
Contributor Author

I pushed a new commit to fix the linting

langchain/embeddings/openai.py Outdated Show resolved Hide resolved
langchain/embeddings/openai.py Outdated Show resolved Hide resolved
@hwchase17 hwchase17 added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 1, 2023
@hwchase17 hwchase17 changed the base branch from master to harrison/azure-openai May 2, 2023 04:24
@hwchase17 hwchase17 merged commit e2e0523 into langchain-ai:harrison/azure-openai May 2, 2023
5 of 9 checks passed
zioproto added a commit to zioproto/cheshire-cat that referenced this pull request May 2, 2023
Do not use the Fake Embeddings anymore.
Requires LangChain 0.0.155 because it contains langchain-ai/langchain#3711
zioproto added a commit to zioproto/cheshire-cat that referenced this pull request May 2, 2023
Do not use the Fake Embeddings anymore.
Requires LangChain 0.0.155 because it contains langchain-ai/langchain#3711
@aakashrshah
Copy link

aakashrshah commented May 2, 2023

Related issue: #3992

It needs to allow headers as well as model_kwargs, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants