weiqingy commented on issue #931:
URL: https://github.com/apache/flink-agents/issues/931#issuecomment-5228873326

   Sounds good. I'll also walk back what I wrote in the issue about the two 
connections not being equivalent. Microsoft's endpoints doc says the Azure 
OpenAI endpoint can also reach non-OpenAI models, and the Cohere / Llama / 
Mistral / Phi models we document `azureai` for are sold by Azure and deployed 
to that same resource. So the OpenAI v1 route covers the surface, and a compat 
layer wouldn't buy anything. 
https://learn.microsoft.com/en-us/azure/foundry/foundry-models/concepts/endpoints
   
   Two things I'll make sure the migration note gets right, since the endpoint 
value changes and not just the key name. The URL form is different, so copying 
the old `endpoint` straight into `api_base_url` won't work: 
`https://<resource>.services.ai.azure.com/models` becomes 
`https://<resource>.openai.azure.com/openai/v1/`, and our current doc example 
is `https://your-resource.inference.ai.azure.com`, which is neither. And 
`model` is now required and has to be the deployment name, where it used to be 
optional. 
https://learn.microsoft.com/en-us/azure/foundry/how-to/model-inference-to-openai-migration
   
   The note will also say why it points at `OpenAICompletionsConnection` rather 
than `AzureOpenAIChatModelConnection`, since we ship both and the one with 
"Azure" in the name is the one that doesn't fit here. The v1 route takes no 
`api-version`.
   
   Beyond the four things you listed, the removal also touches `azure` in 
`Aliases.java` and `aliases.py`, once for connections and once for setups. 
`azure_openai` already points at the Java Azure OpenAI classes in both, so 
dropping `azure` leaves no hole. Then 
`ResourceName.ChatModel.Java.AZURE_CONNECTION` and `AZURE_SETUP` in 
`python/flink_agents/api/resource.py`, which is how Python agents reach it 
cross-language. And four poms: `integrations/chat-models`, `dist`, 
`ide-support`, and `e2e-test/flink-agents-end-to-end-tests-integration`.
   
   One behavior worth flagging rather than fixing. `Aliases.resolveClazz` 
returns the name unchanged when it doesn't recognize it, so an old `clazz: 
azure` won't report an unknown alias. It gets treated as a class name and fails 
at class load instead. The error does name the missing class, so I'd leave the 
loader alone and cover this in the migration note rather than add a special 
case for one removed alias.
   
   I'll open the PR.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to