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

Improve (#356) to support registration of wildcard URLs #359

Merged
merged 4 commits into from
Jun 14, 2023

Conversation

coggsfl
Copy link
Contributor

@coggsfl coggsfl commented Jun 13, 2023

Improve on (#356) issue to support wildcard URLs required by Azure API path structure
Add Azure ChatCompletions test cases including test of custom deployment name

@coggsfl coggsfl marked this pull request as ready for review June 13, 2023 02:45
@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Merging #359 (8c74f50) into master (3f4e3bb) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #359   +/-   ##
=======================================
  Coverage   95.22%   95.22%           
=======================================
  Files          17       17           
  Lines         670      670           
=======================================
  Hits          638      638           
  Misses         22       22           
  Partials       10       10           

@coggsfl
Copy link
Contributor Author

coggsfl commented Jun 13, 2023

@sashabaranov please review. I have a larger PR I'm working on for Azure DALL-E 2 support, however this is a simple change which allows test support for Azure OpenAI endpoints.

@coggsfl
Copy link
Contributor Author

coggsfl commented Jun 14, 2023

@sashabaranov let me know if you have any questions about the need for wildcard support in the test handlers?

If the project is going to provide first class support for Azure OpenAI then support for wildcard handlers is required due to the deployment name being a component of the URL structure : /openai/deployments/{model}/chat/completions?api-version={api_version}

chat_test.go Outdated Show resolved Hide resolved
@vvatanabe
Copy link
Collaborator

LGTM

@coggsfl coggsfl closed this Jun 14, 2023
@coggsfl coggsfl deleted the merge0612 branch June 14, 2023 10:59
@coggsfl coggsfl restored the merge0612 branch June 14, 2023 11:04
@coggsfl coggsfl reopened this Jun 14, 2023
Copy link
Owner

@sashabaranov sashabaranov left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

@sashabaranov sashabaranov merged commit 646989c into sashabaranov:master Jun 14, 2023
6 checks passed
@coggsfl coggsfl deleted the merge0612 branch June 14, 2023 15:22
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.

None yet

4 participants