-
Notifications
You must be signed in to change notification settings - Fork 322
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
support AZure Openai #572
support AZure Openai #572
Conversation
modelscope_agent/llm/openai.py
Outdated
messages=messages, | ||
stop=stop, | ||
stream=True, | ||
stream_options=stream_options, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why azure api and openai api are processed differently, they should be identical in terms of api interfaces (for example, both support stream_options)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This is because the Azure API is not always fully compatible with the OpenAI API. It depends on the
api_version
of the Azure instance being used, which in my case is2024-06-01
(latest version). - When I make a request, it results in an error:
openai.BadRequestError: Error code: 400 - {'error': {'message': "Unknown parameter: 'stream_options'.", 'type': 'invalid_request_error', 'param': 'stream_options', 'code': 'unknown_parameter'}}.
- I reviewed the official request body and did not find the 'stream_options' parameter.
- Therefore, it is recommended to handle the request parameters for Azure OpenAI and OpenAI separately for better compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like it is ineed the case, openai/openai-python#1469
that is very unfortunate.
still, i would recommend putting stream_options into kwargs (only when using native openai api), which would make the code much cleaner: there is not need to duplicate the entire completions.create call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. lgtm
Change Summary
Support Azure openai, reference here
Related issue number
#243
Checklist
pre-commit install
andpre-commit run --all-files
before git commit, and passed lint check.Please review @zzhangpurdue