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 configuration choice of getting an immutable type in code generation for bytes thrift type #346

Open
rogern opened this issue Mar 12, 2021 · 1 comment

Comments

@rogern
Copy link

rogern commented Mar 12, 2021

Thrift type bytes currently gets generated as a java.nio.ByteBuffer. This type is mutable and if not careful defensive copying is used reading it changes it's internal position in the array. It supports asReadonlyBuffer but that can't be used if the bytes are sent via Thrift to clients, I see a java.nio.ReadOnlyBufferException thrown if done like that.

So to make this less painful I'd like Scrooge to support this type to be an io.twitter.util.Buf via configuration. Probably as the shared version.

Our workaround right now is defensive copying on the byte array itself or directly use list<byte> in Thrift but it feels like a workaround to me but i'm pretty new on this...

@mosesn
Copy link
Contributor

mosesn commented Jun 2, 2021

@rogern for the slow response. I think this is a reasonable idea. Something we've discussed internally is being able to annotate fields to influence the way they're generated (eg being able to use specialized classes for certain kinds of maps). That might be a reasonable approach for this kind of patch?

Anyway, I think we would be happy to accept a patch that looks something like this, but we should probably discuss in greater detail what we actually want to do, and at what granularity we want to support it.

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

No branches or pull requests

2 participants