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

Add support for streaming delimited messages #529

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

JoshuaLeivers
Copy link
Contributor

This allows developers to easily dump and load multiple messages
from a stream in a way that is compatible with official
protobuf implementations (such as Java's
MessageLite#writeDelimitedTo(...)).

This also adds compatibility tests between the official Java Protobuf
implementation and the new and existing streaming functions in
Betterproto. These tests stream data such as messages to output
files, have a Java binary read them and then write them back using
the protobuf-java functions, and then read them back in on the
Python side to check that the returned data is as expected. This
checks that the official Java implementation (and so any other
matching implementations) can properly parse outputs from
Betterproto, and vice-versa, ensuring compatibility in these
functions between the two.

The Java binary is built on the fly as part of a Pytest fixture, and
these tests are skipped if any necessary tools (i.e. Java or Maven)
are missing from the host machine, or if the build fails (e.g. due
to an incompatible Java version).

@JoshuaLeivers JoshuaLeivers force-pushed the message-streaming branch 3 times, most recently from 04615a9 to f300790 Compare September 21, 2023 16:32
@JoshuaLeivers
Copy link
Contributor Author

Sorry about the force-pushes, was just quickly fixing up the CI test fails before it could be reviewed properly, will avoid them from now on 🙂

@JoshuaLeivers
Copy link
Contributor Author

This should finish addressing the message streaming part of #482 , would it be possible to get a review for this?

@JoshuaLeivers
Copy link
Contributor Author

Just rebased onto current master

This allows developers to easily dump and load multiple messages
from a stream in a way that is compatible with official
protobuf implementations (such as Java's
`MessageLite#writeDelimitedTo(...)`).
These tests stream data such as messages to output files, have a
Java binary read them and then write them back using the
`protobuf-java` functions, and then read them back in on the Python
side to check that the returned data is as expected. This checks
that the official Java implementation (and so any other matching
implementations) can properly parse outputs from Betterproto, and
vice-versa, ensuring compatibility in these functions between the
two.
@JoshuaLeivers
Copy link
Contributor Author

JoshuaLeivers commented Oct 16, 2023

Had to re-sign the commits

@cetanu
Copy link
Collaborator

cetanu commented Oct 16, 2023

Looks reasonable to me, thanks for this work

@cetanu cetanu merged commit 4f18ed1 into danielgtaylor:master Oct 16, 2023
17 checks passed
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.

None yet

2 participants