-
Notifications
You must be signed in to change notification settings - Fork 364
Added support for AzureAI client service#188
Added support for AzureAI client service#188adityasugandhi wants to merge 16 commits intoSylphAI-Inc:mainfrom
Conversation
Added Support Azure AI, client serivce #92
Now you can use either OPEN_AI_KEY or you can use Azure identity service to directly authenticate to your end points using credentials
|
@adityasugandhi |
|
@hmehta92 I have fixed the azure_ad_token provider call, and line 163 to 180 serves for AzureOpenAI, where line 183 to 200 is for AsyncAzureOpenAI. |
|
Hi, what's the status of this PR? Do you need help @adityasugandhi for anything? I would like to see this feature implemented asap along with AWS Bedrock as they could serve more companies as they usually go with cloud providers when the other part of the cloud is hosted there. |
|
@chainyo I am still waiting for the review from the repo owner. @liyin2015 Sure, I will start looking into AWS Bedrock |
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.
(1) Please add azure as a optional package in lazy_import
(2) please add a test file in adalflow/tests
(3) add azure in the pyproject.toml as an extra package
| chat_completion_parser: Callable[[Completion], Any] = None, | ||
| input_type: Literal["text", "messages"] = "text", | ||
| ): | ||
| r"""It is recommended to set the OPENAI_API_KEY environment variable instead of passing it as an argument. |
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.
so it does not have it only api_key? why using openai_api_key?
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.
@liyin2015 I have made changes as mentioned in the comments
|
@liyin2015 I have made changes |
| log_probs = [] | ||
| for c in completion.choices: | ||
| content = c.logprobs.content | ||
| print(content) |
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.
@adityasugandhi I think you forgot to remove print statement here on line 90.
|
Hey where are we at with this PR? @adityasugandhi |
| @@ -0,0 +1,450 @@ | |||
| """AzureOpenAI ModelClient integration.""" | |||
|
|
|||
| import os | |||
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 feels almost the same as openai client, maybe we should just subclass from openai client and overwrite a few functions. [Will accept for now, but will need a lot more work to simplify]
|
|
||
|
|
||
| openai = safe_import(OptionalPackages.OPENAI.value[0], OptionalPackages.OPENAI.value[1]) | ||
| from azure.identity import DefaultAzureCredential, get_bearer_token_provider |
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.
please change this to safe import
liyin2015
left a comment
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.
Approve for now and will test more later and release in the next version
adalflow/tests/test_AzureClient.py
Outdated
| from openai.types.chat import ChatCompletionChunk, ChatCompletion | ||
| from adalflow.core.model_client import ModelClient | ||
| from adalflow.core.types import ModelType, EmbedderOutput, TokenLogProb, CompletionUsage, GeneratorOutput | ||
| from adalflow.components.model_client.openai_client import AzureAIClient |
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.
all tests are wrong.
liyin2015
left a comment
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 pr is nowhere near to be accepted, and will need to take a more serious investigation and tests and code refactor
Hi @liyin2015 , I have updated the pr and have made fixes as suggested earlier. with test_azure_client.py tested successfully. |