Enable use of Azure OpenAI client #592
Conversation
|
|
|
||
| azure_openapi_key = os.getenv("CODEMODDER_AZURE_OPENAI_API_KEY") | ||
| azure_openapi_endpoint = os.getenv("CODEMODDER_AZURE_OPENAI_ENDPOINT") | ||
| if bool(azure_openapi_key) ^ bool(azure_openapi_endpoint): |
There was a problem hiding this comment.
could use walrus here for 2 lines above
There was a problem hiding this comment.
That's true although I think in this case it might make it a little harder to read 😅
| if not Client: | ||
| logger.debug("OpenAI API client not available") | ||
| def _setup_llm_client(self) -> OpenAI | None: | ||
| if not AzureOpenAI: |
There was a problem hiding this comment.
the PR description is not very descriptive so I'm guessing here but if the goal is to use either OpenAI or AzureOpenAI (or nothing), shouldn't the if not OpenAI: check also be up here and not short circuit ` if not AzureOpenAI:``
There was a problem hiding this comment.
The order of precedence is to check for Azure OpenAI first and then OpenAI as per the spec linked above. However there's not really going to be a case where AzureOpenAI is defined but OpenAI is not. That means the check below for OpenAI is theoretically redundant although it does help satisfy the type checker.
There was a problem hiding this comment.
ah thanks for clarifying. if there isn't such case you could if not AzureOpenAI or OpenAI but it's all good
There was a problem hiding this comment.
That's true, that's probably a bit clearer.
| @@ -77,3 +82,120 @@ def test_failed_dependency_description(self, mocker): | |||
| ```""" | |||
| in description | |||
| ) | |||
|
|
|||
| def test_setup_llm_client_no_env_vars(self, mocker): | |||
There was a problem hiding this comment.
could add some caplog assertions for the logging, maybe that would clear up my misunderstanding above
Overview
Enable use of Azure OpenAI client as alternative to OpenAI
Details