fix: Allow split page logic to process files concurrently #175
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
awalker4
requested review from
jordan-homan,
micmarty-deepsense and
amadeusz-ds
badGarnet
reviewed
Sep 17, 2024
badGarnet
reviewed
Sep 17, 2024
badGarnet
approved these changes
Sep 17, 2024
yuming-long
added a commit
to Unstructured-IO/unstructured-js-client
that referenced
this pull request
Oct 2, 2024
### Summary copy Unstructured-IO/unstructured-python-client#175 into JS/TS ### Test added a integration tests that is passed on CI Local integration test: * docker start core product api but with -p 8000:5000 (change the line in make file for make docker-start-api) * `make build` * `npx jest --verbose --detectOpenHandles --config jest.config.js test/integration --forceExit -t "SplitPDF async can be used to send multiple files concurrently"` you can also move the unit test to main and `make build npx jest test...` again and will see the test fail, but not on this branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Issue
split_pdf_hook.pydoes not support multiple concurrent files. This is because we store the split request tasks inself.coroutines_to_execute[operation_id], whereoperation_idis just the string "partition". Therefore, if we send two concurrent docs using the same SDK, they'll both try to await the same list of coroutines. This could result in interleaved results, but mostly it breaks withRuntimeError: coroutine is being awaited already, as the second request gets ready to await its requests. This will block anyone trying to use the newpartition_asyncto fan out their pdfs.Note that the js/ts client also has this issue.
The fix
We need to use an actual id to index into
coroutines_to_execute. Inbefore_request, let's make a uuid and build up the list of coroutines for this doc. We need to pass this id toafter_successin order to retrieve the results, so we can set it as a header on our "dummy" request that's returned to the SDK.Testing
See the new integration test. We can verify this by sending two docs serially, and then with
asyncio.gather, and confirm that the results are the same.