[azure] only add deployment in url path for deployment-based APIs #8
Conversation
src/openai/lib/azure.py
Outdated
| if "/deployments" in str(self.base_url.path) and url not in _deployments_endpoints: | ||
| merge_url = httpx.URL(url) | ||
| if merge_url.is_relative_url: | ||
| merge_path = f"{self.base_url.path.rsplit('/deployments', maxsplit=1)[0]}/{merge_url.path.lstrip('/')}" |
There was a problem hiding this comment.
Rather than adding it and then removing it, did we consider adding it only when needed?
There was a problem hiding this comment.
Yes, that would be the better way of dealing with it. However, we'd have to change the current value of client.base_url to take that approach which could break users.
There was a problem hiding this comment.
Should we split on/find '/deployments/'? I guess it is unlikely that one would name a deployment "deployments" (and we do a maxsplit=1 so it is only a problem if we have a deployments "to the right" of the intended split point).
There was a problem hiding this comment.
Sure, the base_url property enforces a trailing slash so it will always be there, regardless of whether the user added it.
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "client,base_url,api_path,json_data,expected", |
There was a problem hiding this comment.
I think we can come up with a couple of additional tests here - e.g. have a dns name with deployments in it, add a deployment called deployments, add a couple of deployments to the azure_endpoint url etc.
3 participants
No description provided.