-
Notifications
You must be signed in to change notification settings - Fork 52
fix: Prepare for breaking change in upcoming unstructured-client #379
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
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
|
|
||
| # 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_urlis 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_urlper endpoint.Note that for compatibility with the current version, we need to strip the
/general/v0/generalpath 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_urlpassed 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