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.

Sign up for GitHub

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

Jump to bottom

embeddings/azure-openai: Fixing issues with azure openai implementation #253

Merged
merged 6 commits into from
Aug 18, 2023
Merged

embeddings/azure-openai: Fixing issues with azure openai implementation #253

merged 6 commits into from
Aug 18, 2023

Conversation

nidzola
Copy link
Contributor

@nidzola nidzola commented Aug 17, 2023 •
edited
Loading

PR Checklist

  • Read the Contributing documentation.
  • Read the Code of conduct documentation.
  • Name your Pull Request title clearly, concisely, and prefixed with the name of the primarily affected package you changed according to Good commit messages (such as memory: add interfaces for X, Y or util: add whizzbang helpers).
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. Fixes #123).
  • Describes the source of new concepts.
  • References existing implementations as appropriate.
  • Contains test coverage for new functions.
  • Passes all golangci-lint checks.

Description

  • if we want to use azure openai implementation, we need to provide the needed embedder model (aka deployment in the Azure portal), this is the required parameter in the Azure documentation
  • each deployment in azure is used for diff purpose
  • fixing the issue and adding the tests

Screenshot

Screenshot 2023-08-17 at 7 55 36 PM

@tmc tmc merged commit fef0821 into tmc:main Aug 18, 2023
3 checks passed
ClaudiaJ added a commit to ClaudiaJ/zep that referenced this pull request Aug 25, 2023
@ClaudiaJ
fix/AuzreOpenAI embeddings and LLM deployment name
f29baf5 
solves "The API deployment for this resource does not exist" for LLM and
embedding models deployed in Azure OpenAI by deployment name supported
in tmc/langchaingo#253

We can't Validate OpenAI LLM model names from hard-coded list in Azure
because the model name parameter in API request is a deployment name,
and while Microsoft advises us to use the model name as deployment name,
we did not listen, and I didn't want to coordinate redeploying with a
different name on a Friday.

This also permits use of customized models that can be deployed in Azure
side-by-side base models as added benefit so I think it was worthwhile.
@ClaudiaJ ClaudiaJ mentioned this pull request Aug 25, 2023
Closed
danielchalef pushed a commit to getzep/zep that referenced this pull request Aug 28, 2023
@ClaudiaJ @danielchalef
fix/AuzreOpenAI embeddings and LLM deployment name
fed860b 
solves "The API deployment for this resource does not exist" for LLM and
embedding models deployed in Azure OpenAI by deployment name supported
in tmc/langchaingo#253

We can't Validate OpenAI LLM model names from hard-coded list in Azure
because the model name parameter in API request is a deployment name,
and while Microsoft advises us to use the model name as deployment name,
we did not listen, and I didn't want to coordinate redeploying with a
different name on a Friday.

This also permits use of customized models that can be deployed in Azure
side-by-side base models as added benefit so I think it was worthwhile.
@danielchalef danielchalef mentioned this pull request Aug 28, 2023
Merged
danielchalef added a commit to getzep/zep that referenced this pull request Aug 28, 2023
@danielchalef @ClaudiaJ
fix/AuzreOpenAI embeddings and LLM deployment name (#172)
cbf4fe4 
solves "The API deployment for this resource does not exist" for LLM and
embedding models deployed in Azure OpenAI by deployment name supported
in tmc/langchaingo#253

We can't Validate OpenAI LLM model names from hard-coded list in Azure
because the model name parameter in API request is a deployment name,
and while Microsoft advises us to use the model name as deployment name,
we did not listen, and I didn't want to coordinate redeploying with a
different name on a Friday.

This also permits use of customized models that can be deployed in Azure
side-by-side base models as added benefit so I think it was worthwhile.

Co-authored-by: Claudia Justice Kane <claudia.hardman@mattel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nidzola @tmc @zivkovicn