microsoft
Add AsyncMarkItDown as a wrapper
My take on how this code should support async, given all underlying libraries are not supporting async.
For more details/context, see: https://github.com/microsoft/markitdown/issues/13#issuecomment-2543834157
@0xRaduan -- thanks for the PR.
@afourney @jackgerrits @0xRaduan, I love this PR, but I am worried about the code duplication because of asynchronous tests. What if we stuck with sync implementation, but added an FAQ showing how a user could create an asynchronous wrapper similar to your approach.
This was users know how to create an asynchronous wrapper, but we avoid the code/test duplication? Please lmk your thoughts.
Hey @gagb - good feedback, I agree.
In general, this async code is just a wrapper over sync code [1], so technically you don't need to have a full suite of async tests, as all of the functionality should be tested in normal tests.
I would suggest keeping just one async test to verify that the wrapper works, and remove everything else. Initially I've migrated all tests to just verify that it works identically for my own's sake.
Hey @gagb - good feedback, I agree.
In general, this async code is just a wrapper over sync code [1], so technically you don't need to have a full suite of async tests, as all of the functionality should be tested in normal tests.
I would suggest keeping just one async test to verify that the wrapper works, and remove everything else. Initially I've migrated all tests to just verify that it works identically for my own's sake.
I like your framing. Would love @jackgerrits feedback on the strategy.
@gagb - fyi, updated the code, fixed linter, removed all tests and just kept one async test to make sure wrapper works in async environment.
@markthepixel - let me take a look tomorrow, sorry for the wait!
(actually needs this myself, so will take a look asap)
I gatta be honest I thought this was my issue but it turns out when you feed a string in instead of a path/link it just gives a vague error 😆