Skip to content
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

Closed
ntninja opened this issue Oct 16, 2019 · 10 comments
Closed

Make this library async-friendly #8

ntninja opened this issue Oct 16, 2019 · 10 comments

Comments

@ntninja
Copy link
Collaborator

ntninja commented Oct 16, 2019

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. Using anyio for this would likely be “good enough” and not require us to bind ourselves to one specific event loop while the asyncio vs trio debate is still ongoing.

@ntninja
Copy link
Collaborator Author

ntninja commented Oct 18, 2019

Looks like anyio currently does not provide support for any kind of asynchronous I/O streams. Without those, using it will become quite verbose and add lots of boilerplate code.

After contemplating, I think going with trio for now should be OK. Once we have a working setup with one AIO library, translating the code to another shouldn't be too much work.

@AliabbasMerchant: Any opinions on this?

@AliabbasMerchant
Copy link
Member

@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.

@ntninja
Copy link
Collaborator Author

ntninja commented Oct 24, 2019

Bump. I've now converted the entire py-datastore core to async using trio, so far our dependence on trio is rather weak and changing the asyncio framework would mostly consist of renaming a few methods and changing some imports. But the more I convert the harder it will likely get, so it would be useful to have a decision very soon.

@AliabbasMerchant
Copy link
Member

Lets go with trio

@AliabbasMerchant
Copy link
Member

@Alexander255 Umm... (Did you forget to create a PR? Or is it still WIP?)

@ntninja
Copy link
Collaborator Author

ntninja commented Oct 25, 2019

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.

@AliabbasMerchant
Copy link
Member

Type annotations are awesome!

@ntninja
Copy link
Collaborator Author

ntninja commented Nov 10, 2019

See #12

@adamjsawicki
Copy link

@Alexander255 can this be closed because of #12?

@ntninja
Copy link
Collaborator Author

ntninja commented Jan 27, 2020

@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? 🙂

@ntninja ntninja closed this as completed Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants