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

Missing ability to convert akka.protobuf.ByteString into akka.util.ByteString #18738

Open
jypma opened this Issue Oct 19, 2015 · 5 comments

Comments

Projects
None yet
5 participants
@jypma
Member

jypma commented Oct 19, 2015

We're implementing an akka persistence query plugin. It needs to look inside stored events that are serialized using akka's built-in serialization, in this case the protobuf representation of akka.persistence.serialization.MessageFormats.PersistentMessage. For ease of use, we're re-using that class to unmarshal the persistent events.

PersistentMessage.payload is an instance of akka.protobuf.ByteString, which contains the serialized event itself. The contents depends on how the user has configured akka-serialization: it might be java serialization, a custom probuf version, or something else entirely.

One of the use cases of our plugin is to allow remote views for events. To that end, we actually want to expose the contents of the events directly. Hence, we don't need to read the contents; we just want to serve it up to HTTP (as chunks).

However, akka-http expects akka.util.ByteString, not akka.protobuf.ByteString. Even worse is that with current APIs, it's not possible to convert one into the other without copying the bytes. It would be very convenient to be allowed to coerce protobuf.ByteString into util.ByteString, while re-using the underlying byte array / byte buffer.

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Oct 19, 2015

Member

// cc @rkuhn since you've been playing around with ByteString compat things recently AFAIR?

Member

ktoso commented Oct 19, 2015

// cc @rkuhn since you've been playing around with ByteString compat things recently AFAIR?

@ktoso ktoso added this to the 2.4.x milestone Oct 19, 2015

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Oct 19, 2015

Member

To me it sounds a bit weird that we're emitting the protobuf.ByteString there... will need to investigate.
Thanks for reporting @jypma !

Member

ktoso commented Oct 19, 2015

To me it sounds a bit weird that we're emitting the protobuf.ByteString there... will need to investigate.
Thanks for reporting @jypma !

@rkuhn

This comment has been minimized.

Show comment
Hide comment
@rkuhn

rkuhn Oct 19, 2015

Collaborator

For the shaded protobuf version we could indeed add compat with our own ByteString, but we should also make sure that we’re not using the wrong one in places.

Collaborator

rkuhn commented Oct 19, 2015

For the shaded protobuf version we could indeed add compat with our own ByteString, but we should also make sure that we’re not using the wrong one in places.

@viktorklang

This comment has been minimized.

Show comment
Hide comment
@viktorklang

viktorklang Oct 29, 2015

Member

Why not investigate whether we can remove akka.protobuf.ByteString and have it use akka.util.ByteString instead?

Member

viktorklang commented Oct 29, 2015

Why not investigate whether we can remove akka.protobuf.ByteString and have it use akka.util.ByteString instead?

@patriknw

This comment has been minimized.

Show comment
Hide comment
@patriknw

patriknw Mar 18, 2016

Member

Patching the internal Protobuf library too much feels dangerous because we don't want to be maintainer of protobuf, i.e. it would be difficult to upgrade. Adding a simple conversion method sounds alright though.

Member

patriknw commented Mar 18, 2016

Patching the internal Protobuf library too much feels dangerous because we don't want to be maintainer of protobuf, i.e. it would be difficult to upgrade. Adding a simple conversion method sounds alright though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment