-
Notifications
You must be signed in to change notification settings - Fork 8
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
Make this library async-friendly #8
Comments
Looks like After contemplating, I think going with @AliabbasMerchant: Any opinions on this? |
@Alexander255 To be honest, I've never properly worked with any Async I/O library in Python. So no idea. Let me check out a few of the libraries. |
Bump. I've now converted the entire |
Lets go with |
@Alexander255 Umm... (Did you forget to create a PR? Or is it still WIP?) |
Ok thanks! I must admit that my codebase right now has not only the async/await port, but also the complete ObjectDatastore/BinaryDatastore split – so it's gonna be quite a large PR, I fear. I couldn't complete it yesterday though, as I got carried away adding typing annotations to some stuff that apparently cannot be represented by mypy at this time. (It was useful for finding a bunch of other inconsistencies before this however, so adding annotations wasn't just a waste of time like in this latest case involving cooperative multiple inheritance.) I'll try to open a PR today though. |
Type annotations are awesome! |
See #12 |
@Alexander255 can this be closed because of #12? |
@swixx: Yes, although the FS datastore provider hasn't been updated yet in master (see #13). @AliabbasMerchant @seenivasanseeni: May I have a review on #13 please? 🙂 |
Currently all of this library assumes synchronous I/O, considering that that is bad practice when interacting with the network datastore implementations and doesn't align with our goals for
py-ipfs
. Usinganyio
for this would likely be “good enough” and not require us to bind ourselves to one specific event loop while theasyncio
vstrio
debate is still ongoing.The text was updated successfully, but these errors were encountered: