-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Prepare for breaking change in upcoming unstructured-client #379
Conversation
In the unreleased 0.30.0 of `unstructured-client`, handling of the `server_url` is changing. We need to do this to integrate the platform API functions. Now, different parts of the SDK are talking to different backend services, and so it's preferred that we set the `server_url` per endpoint. Note that for compatibility with the current SDK, we need to strip the `/general/v0/general` path off of the URL. We have logic to do this in the SDK, but in versions before 0.30.0 this doesn't take effect when you pass the URL in the endpoint like this.
1b0a8c4
to
02cddf4
Compare
@@ -87,13 +87,16 @@ async def call_api_async( | |||
""" | |||
from unstructured_client import UnstructuredClient | |||
|
|||
# Note(austin) - the sdk takes the base url, but users may pass the full endpoint | |||
# For consistency, strip off the path when it's given | |||
base_url = server_url[:-19] if "/general/v0/general" in server_url else server_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be safer to split on /general/v0/general
in case there are trailing spaces or tabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also server_url
is an optional parameter, so it would make sense to check if it exists.
Closing in preference of an upstream fix: Unstructured-IO/unstructured-python-client#226 |
Pull request was closed
In the unreleased 0.30.0 of
unstructured-client
, handling of theserver_url
is changing. We need to do this to integrate the platform API functions. Now, different parts of the SDK are talking to different backend services, and so it's preferred that we set theserver_url
per endpoint.Note that for compatibility with the current version, we need to strip the
/general/v0/general
path off of the URL. We have logic to do this in the SDK, but in versions before 0.30.0 this only applies to theserver_url
passed to the client instance. So, it's a bit of a chicken and egg. Once 0.30.0 is published and pulled in this repo, we can remove these lines if we want.Related core library fix: Unstructured-IO/unstructured#3916