Skip to content

Commit b0a005f

Browse files
awalker4awalker4
authored
chore: Revert custom handling for multiple Unstructured base urls (#270)
# The Issue We discovered a behavior change in the Python SDK after we merged the platform/serverless api specs. All of a sudden, the SDK level server_url param silently stopped working, and we were forced to set custom urls per function. ``` # The passed url is ignored and we go to our default Serverless URL. # You suddenly get an invalid api key error if you expected to talk to, e.g, freemium s = UnstructuredClient(server_url="my_own_url") s.general.partition() # This does the right thing s = UnstructuredClient() s.general.partition(server_url="my_own_url") ``` We had to patch some generated code in order to keep backwards compatibility, and set the SDK level `server_url` the way we used to. This works! However, any file in `.genignore` will not get updated and eventually the SDK fails to generate because of drift. The better solution is to figure out why the generated code changed on us, and fix it "upstream". # The Fix Our SDK points to two services - the workflow API at `platform.unstructuredapp.io` and the older partition endpoint at `api.unstructuredapp.io`. We merged these two openapi specs in order to generate a combined SDK, but this meant that urls could only be resolved per operation. There is no longer a global default, so a statement like `UnstructuredClient(server_url="my_own_url")` is ambiguous. The solution to all this is to go back to one default server - the platform url. The partition url is just one endpoint so it's much easier to handle as a one off. This restores the `server_url` behavior we had, without us having to fight with the autogenerated code. # The Diff This pr is huge because I regenerated the relevant files. There are only a few changes that drive all of it: ## `overlay_client.yaml` After merging the two `openapi.yaml` specs, remove all child `servers` blocks and just keep one global config. Now every endpoint is a part of `platform.unstructuredapp.io` ## `general.py` This is now the only custom patch. In the `partition` (and `partition_async`) call, we need to swap to the right url. We do this only if the user has not already changed the default. ## `destinations.py`, `jobs.py`, etc These are the other endpoint files that are no longer patched. After regenerating, you can see the `base_url` logic cleans itself up. Either the user passed a `server_url` in the call, or we fetch the globally configured url. ## `test_server_urls.py` Made some tweaks to these test cases. This locks in our compatibility and asserts that we always use the right url. Users can set a custom url at the SDK init, or at the operation. We need to cover this behavior within `general.partition` since this has the special logic. Otherwise, make sure both url approaches work for any of the other platform operations.
1 parent 1363a12 commit b0a005f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+335
-583
lines changed

‎.genignore‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,10 @@ _test_unstructured_client
88
# ignore Makefile
99
Makefile
1010

11-
# Ignore the base resource classes while we're patching in our own server url logic
12-
# If we add a new endpoint, we need to:
13-
# - Comment out the ignore for the right file
11+
# Ignore general.py so we can patch in our partitioning url
12+
# If we ever have a new endpoint under /general, we need to:
13+
# - Comment out this ignore line
1414
# - Generate locally, watch the new endpoint appear
15+
# - Adjust the custom url snippets in the file
1516
# - Bring back the ignore line and commit
16-
src/unstructured_client/destinations.py
1717
src/unstructured_client/general.py
18-
src/unstructured_client/jobs.py
19-
src/unstructured_client/sources.py
20-
src/unstructured_client/workflows.py

‎_test_unstructured_client/unit/test_server_urls.py‎

Lines changed: 15 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -67,61 +67,33 @@ class URLTestCase:
6767
"case",
6868
[
6969
URLTestCase(
70-
description="non UNST domain client-level URL, no path",
70+
description="custom client-level URL, no path",
7171
sdk_endpoint_name="general.partition_async",
7272
client_url="http://localhost:8000/",
7373
endpoint_url=None,
7474
expected_url="http://localhost:8000"
7575
),
7676
URLTestCase(
77-
description="non UNST domain client-level URL, with path",
77+
description="custom client-level URL, with path",
7878
sdk_endpoint_name="general.partition_async",
7979
client_url="http://localhost:8000/my/endpoint/",
8080
endpoint_url=None,
8181
expected_url="http://localhost:8000/my/endpoint"
8282
),
8383
URLTestCase(
84-
description="non UNST domain endpoint-level URL, no path",
84+
description="custom endpoint-level URL, no path",
8585
sdk_endpoint_name="general.partition_async",
8686
client_url=None,
8787
endpoint_url="http://localhost:8000/",
8888
expected_url="http://localhost:8000"
8989
),
9090
URLTestCase(
91-
description="non UNST domain endpoint-level URL, with path",
91+
description="custom endpoint-level URL, with path",
9292
sdk_endpoint_name="general.partition_async",
9393
client_url=None,
9494
endpoint_url="http://localhost:8000/my/endpoint/",
9595
expected_url="http://localhost:8000/my/endpoint"
9696
),
97-
URLTestCase(
98-
description="UNST domain client-level URL, no path",
99-
sdk_endpoint_name="general.partition_async",
100-
client_url="https://unstructured-000mock.api.unstructuredapp.io",
101-
endpoint_url=None,
102-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
103-
),
104-
URLTestCase(
105-
description="UNST domain client-level URL, with path",
106-
sdk_endpoint_name="general.partition_async",
107-
client_url="https://unstructured-000mock.api.unstructuredapp.io/my/endpoint/",
108-
endpoint_url=None,
109-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
110-
),
111-
URLTestCase(
112-
description="UNST domain endpoint-level URL, no path",
113-
sdk_endpoint_name="general.partition_async",
114-
client_url=None,
115-
endpoint_url="https://unstructured-000mock.api.unstructuredapp.io",
116-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
117-
),
118-
URLTestCase(
119-
description="UNST domain endpoint-level URL, with path",
120-
sdk_endpoint_name="general.partition_async",
121-
client_url=None,
122-
endpoint_url="https://unstructured-000mock.api.unstructuredapp.io/my/endpoint/",
123-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
124-
),
12597
URLTestCase(
12698
description="default URL fallback",
12799
sdk_endpoint_name="general.partition_async",
@@ -161,63 +133,49 @@ async def test_async_endpoint_uses_correct_url(monkeypatch, case: URLTestCase):
161133
"case",
162134
[
163135
URLTestCase(
164-
description="non UNST domain client-level URL, no path",
136+
description="custom client-level URL, no path",
165137
sdk_endpoint_name="destinations.create_destination",
166138
client_url="http://localhost:8000/",
167139
endpoint_url=None,
168140
expected_url="http://localhost:8000"
169141
),
170142
URLTestCase(
171-
description="non UNST domain client-level URL, with path",
143+
description="custom client-level URL, with path",
172144
sdk_endpoint_name="sources.create_source",
173145
client_url="http://localhost:8000/my/endpoint/",
174146
endpoint_url=None,
175147
expected_url="http://localhost:8000/my/endpoint"
176148
),
177149
URLTestCase(
178-
description="non UNST domain endpoint-level URL, no path",
150+
description="custom endpoint-level URL, no path",
179151
sdk_endpoint_name="jobs.get_job",
180152
client_url=None,
181153
endpoint_url="http://localhost:8000",
182154
expected_url="http://localhost:8000"
183155
),
184156
URLTestCase(
185-
description="non UNST domain endpoint-level URL, with path",
157+
description="custom endpoint-level URL, with path",
186158
sdk_endpoint_name="workflows.create_workflow",
187159
client_url=None,
188160
endpoint_url="http://localhost:8000/my/endpoint",
189161
expected_url="http://localhost:8000/my/endpoint"
190162
),
191163
URLTestCase(
192-
description="UNST domain client-level URL, no path",
164+
description="partition client level with path",
193165
sdk_endpoint_name="general.partition",
194-
client_url="https://unstructured-000mock.api.unstructuredapp.io",
166+
client_url="https://api.unstructuredapp.io/general/v0/general",
195167
endpoint_url=None,
196-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
197-
),
198-
URLTestCase(
199-
description="UNST domain client-level URL, with path",
200-
sdk_endpoint_name="general.partition",
201-
client_url="https://unstructured-000mock.api.unstructuredapp.io/my/endpoint/",
202-
endpoint_url=None,
203-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
204-
),
205-
URLTestCase(
206-
description="UNST domain endpoint-level URL, no path",
207-
sdk_endpoint_name="general.partition",
208-
client_url=None,
209-
endpoint_url="https://unstructured-000mock.api.unstructuredapp.io",
210-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
168+
expected_url="https://api.unstructuredapp.io"
211169
),
212170
URLTestCase(
213-
description="UNST domain endpoint-level URL, with path",
171+
description="partition endpoint level with path",
214172
sdk_endpoint_name="general.partition",
215173
client_url=None,
216-
endpoint_url="https://unstructured-000mock.api.unstructuredapp.io/my/endpoint/",
217-
expected_url="https://unstructured-000mock.api.unstructuredapp.io"
174+
endpoint_url="https://api.unstructuredapp.io/general/v0/general",
175+
expected_url="https://api.unstructuredapp.io"
218176
),
219177
URLTestCase(
220-
description="default URL fallback",
178+
description="partition default url",
221179
sdk_endpoint_name="general.partition",
222180
client_url=None,
223181
endpoint_url=None,

‎docs/sdks/destinations/README.md‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ with UnstructuredClient() as uc_client:
4242
| -------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |
4343
| `request` | [operations.CreateConnectionCheckDestinationsRequest](../../models/operations/createconnectioncheckdestinationsrequest.md) | :heavy_check_mark: | The request object to use for the request. |
4444
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
45-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
4645

4746
### Response
4847

@@ -93,7 +92,6 @@ with UnstructuredClient() as uc_client:
9392
| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ |
9493
| `request` | [operations.CreateDestinationRequest](../../models/operations/createdestinationrequest.md) | :heavy_check_mark: | The request object to use for the request. |
9594
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
96-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
9795

9896
### Response
9997

@@ -135,7 +133,6 @@ with UnstructuredClient() as uc_client:
135133
| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ |
136134
| `request` | [operations.DeleteDestinationRequest](../../models/operations/deletedestinationrequest.md) | :heavy_check_mark: | The request object to use for the request. |
137135
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
138-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
139136

140137
### Response
141138

@@ -177,7 +174,6 @@ with UnstructuredClient() as uc_client:
177174
| -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- |
178175
| `request` | [operations.GetConnectionCheckDestinationsRequest](../../models/operations/getconnectioncheckdestinationsrequest.md) | :heavy_check_mark: | The request object to use for the request. |
179176
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
180-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
181177

182178
### Response
183179

@@ -219,7 +215,6 @@ with UnstructuredClient() as uc_client:
219215
| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ |
220216
| `request` | [operations.GetDestinationRequest](../../models/operations/getdestinationrequest.md) | :heavy_check_mark: | The request object to use for the request. |
221217
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
222-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
223218

224219
### Response
225220

@@ -259,7 +254,6 @@ with UnstructuredClient() as uc_client:
259254
| ---------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- |
260255
| `request` | [operations.ListDestinationsRequest](../../models/operations/listdestinationsrequest.md) | :heavy_check_mark: | The request object to use for the request. |
261256
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
262-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
263257

264258
### Response
265259

@@ -311,7 +305,6 @@ with UnstructuredClient() as uc_client:
311305
| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ |
312306
| `request` | [operations.UpdateDestinationRequest](../../models/operations/updatedestinationrequest.md) | :heavy_check_mark: | The request object to use for the request. |
313307
| `retries` | [Optional[utils.RetryConfig]](../../models/utils/retryconfig.md) | :heavy_minus_sign: | Configuration to override the default retry behavior of the client. |
314-
| `server_url` | *Optional[str]* | :heavy_minus_sign: | An optional server URL to use. |
315308

316309
### Response
317310

0 commit comments

Comments
 (0)