microsoft / semantic-kernel

Integrate cutting-edge LLM technology quickly and easily into your apps
https://aka.ms/semantic-kernel
MIT License
21.72k stars 3.22k forks source link

Python: Extra slashes are inserted in endpoint #5236

Closed ymuichiro closed 7 months ago

ymuichiro commented 7 months ago

Describe the bug Extra slashes are inserted in endpoint. openai-python receives endpoint as a string and adds one slash.

f"{azure_endpoint}/openai/deployments/{azure_deployment}"
# {azure_endpoint} >>/<< openai

https://github.com/openai/openai-python/blob/e41abf7b7dbc1e744d167f748e55d4dedfc0dca7/src/openai/lib/azure.py#L194

However, Semantic Kernel is passing it as https://domain/.

azure_endpoint=str(endpoint)

https://github.com/microsoft/semantic-kernel/blob/739e7f0e2a9f8bbba3d537d8435ff3cc2e294dc7/python/semantic_kernel/connectors/ai/open_ai/services/azure_config_base.py#L82C21-L82C49

This endpoint is the pydantic.networks URL. str(Url) on Url adds a / to the end of the url string.

To Reproduce Pass the url as http://127.0.0.1 to AzureChatCompletion. Then, it becomes http://127.0.0.1//openai/deployments/~.

Expected behavior

OK: endpoint ... http://127.0.0.1/openai/deployments/~ NG: endpoint ... http://127.0.0.1//openai/deployments/~

Screenshots none

Additional context I would like to make the following suggestions.

azure_endpoint=str(endpoint).rstrip("/")
eavanvalkenburg commented 7 months ago

@ymuichiro does this actual cause the url to not work?

ymuichiro commented 7 months ago

@eavanvalkenburg Yes, Azure OpenAI has no problem with such a path, but if you connect to an API compatible with Azure OpenAI (for example, LLama-Cpp-Python), the connection may fail.

Although it may not be the recommended usage, some Azure OpenAI API compatible servers may fail if the URL format is incorrect.

moonbox3 commented 7 months ago

5237 is merged. Closing issue.