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

Allow reading and writing messages from sockets in async mode #313

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

LasseBlaauwbroek
Copy link
Contributor

@LasseBlaauwbroek LasseBlaauwbroek commented Apr 20, 2023

Resolves #298 (see also capnproto/capnproto#1542). Step towards #311.

This PR build on top of #310, only the last commit is relevant.

@kentonv I'd also like to implement read_async_packed() and write_async_packed() if possible. But I don't see any obvious functions in the C++ implementation that make this possible. And I can find surprisingly little information from other people that may have tried this before. The most relevant I can find is https://groups.google.com/g/capnproto/c/X344P1vATsU/m/5qxvGtDvAQAJ How would you suggest this could be implemented? Or is there currently no way? Thanks.

@kentonv
Copy link
Member

kentonv commented Apr 21, 2023

When I read packed encoding in async mode in C++, I do it by reading the whole packed message into memory in advance, then parsing it (from memory) using the synchronous implementation. Similarly for writing, write to a buffer first.

@LasseBlaauwbroek
Copy link
Contributor Author

LasseBlaauwbroek commented Apr 21, 2023

Okay, so presumably you propose to populate a BufferedInputStream with a packed message, which is then consumed by PackedMessageReader? If so, it looks to me like the only way of achieving this in async mode is to have some kind of framing around messages? Otherwise one would not know how large the packed message is. And the message cannot be read in small pieces, because the interface to do this (like BufferedInputStream::tryGetBuffer) is not async. Am I missing something here?

If my analysis is right, isn't this a limitation of the API? Theoretically, async reading of packed messages seems possible (if, perhaps, somewhat slow). It's just the API that doesn't allow it?

@kentonv
Copy link
Member

kentonv commented Apr 21, 2023

Yeah to be honest I've just never used packed encoding in an async stream with multiple messages. It does seem like the library is missing something there.

@haata
Copy link
Collaborator

haata commented Jun 7, 2023

@LasseBlaauwbroek please rebase, otherwise it looks good.

@LasseBlaauwbroek LasseBlaauwbroek changed the base branch from master to test June 7, 2023 18:11
@LasseBlaauwbroek LasseBlaauwbroek changed the base branch from test to master June 7, 2023 18:11
@LasseBlaauwbroek
Copy link
Contributor Author

Bad git for not resolving these conflicts automatically. Rebased.

@haata haata merged commit 8feb377 into capnproto:master Jun 7, 2023
tobiasah added a commit to tobiasah/pycapnp that referenced this pull request Oct 13, 2023
Since capnproto#313 it is possible to read and write messages over a socket.
This commit adds a small section for read and write in the quickstart.
tobiasah added a commit to tobiasah/pycapnp that referenced this pull request Oct 13, 2023
Since capnproto#313 it is possible to read and write messages over a socket.
This commit adds a small section for read and write in the quickstart.
haata pushed a commit that referenced this pull request Oct 16, 2023
* Update documentation to async code (#331)

This commit updates the documentation to the latest changes added
with pycapnp 2.0.0.

* Remove non existing classes/functions from the reference documentation
* Adapt the quickstart to the latest changes. Mainly to new rpc handling,
  that now exlusively is done through asyncio.

* DOC: Add section about send and receive messages over a socket

Since #313 it is possible to read and write messages over a socket.
This commit adds a small section for read and write in the quickstart.
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

Successfully merging this pull request may close these issues.

Consider using Cysignals
3 participants