MAINT: Adding URL validation for openai responses targets #1148
Conversation
| Prints a warning if the endpoint doesn't match any of the expected routes. | ||
| This validation helps ensure the endpoint is configured correctly for the specific API. | ||
| """ | ||
| if not self._endpoint or not self._expected_route: |
There was a problem hiding this comment.
expected route isn't really specific to the instance of a class, but the class itself. As such, we should set it as a constant on the class and pass it as an argument into this method
EXPECTED_URL_REGEX = ...
...
def _warn_if_irregular_endpoint(self, expected_url_regex):
....
|
|
||
| # Check if the endpoint matches any of the expected routes | ||
| for expected_route in expected_routes: | ||
| if expected_route is None: |
There was a problem hiding this comment.
None? Why would that be in there?
| normalized_route = parsed_url.path.lower().rstrip("/") | ||
|
|
||
| # Handle both single route (string) and multiple routes (list) | ||
| expected_routes = self._expected_route if isinstance(self._expected_route, list) else [self._expected_route] |
There was a problem hiding this comment.
why not allow lists of 1 element instead of the special case?
| if "*" in expected_route: | ||
| if self._matches_wildcard_pattern(normalized_route, expected_route): | ||
| return | ||
| else: | ||
| # Exact matching for routes without wildcards | ||
| if normalized_route == expected_route: |
There was a problem hiding this comment.
wouldn't regex matching be easier here?
| else f"one of: {', '.join(self._expected_route)}" | ||
| ) | ||
| logger.warning( | ||
| f"Expected endpoint to end with {expected_routes_str} " |
There was a problem hiding this comment.
| f"Expected endpoint to end with {expected_routes_str} " | |
| f"Expected endpoint to end with {expected_routes_str} " |
| ) | ||
| logger.warning( | ||
| f"Expected endpoint to end with {expected_routes_str} " | ||
| f"Please verify your endpoint URL: '{self._endpoint}'." |
There was a problem hiding this comment.
also pointing to our env_example can help
| self._model_name = "tts-1" | ||
|
|
||
| # Set expected route for URL validation | ||
| self._expected_route = "/openai/deployments/*/audio/speech" |
There was a problem hiding this comment.
Wherever we have AOAI patterns we should also have OAI ones. For Sora it should include both sora1 and sora2
There was a problem hiding this comment.
I think just /audio/speech should be good and cover both?
3 participants
Description
Our endpoints need the URL to end in the correct ending. This adds a warning so users can be aware of this and better debug since the default error message is a harder to understand error 500. This way the warning can point users to check their endpoint URL (see bug here: #1144)
Closes #1144
Tests:
Added unit tests