-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
protoc-gen-go: support go_tag option to specify custom struct tags #52
Comments
No, there's no support for that, nor any intention to support that. |
I'd like to add that this feature would be extremely useful for validation purposes. |
+1 Use case: Using protobuf generated structs along with sqlx package. Would be awesome |
Likewise the go library for cloud datastore. Basically, increasing numbers of things make use of struct tags, and without support for them you're left manually duplicating structs? If there are no plans to add this functionality, what is the recommended way to handle it? |
I would love this feature! |
I would love this feature aswell! |
This would be handy, as I need to validate JSON payloads for my REST API portion of the service. |
Since golang/protobuf will probably never support this. Here are some solutions. https://github.com/mwitkow/go-proto-validators is useful for validation on proto structs. https://github.com/gogo/protobuf can be used in two ways to have custom tags:
|
Need it as well to automate insert/update into the sql database |
I've add a plugin retag for protoc-gen-go. https://github.com/qianlnk/protobuf/tree/master/protoc-gen-go/retag protoc --go_out=plugins=grpc+retag:. yourproto.proto |
@qianlnk .thanks.I tried your plugin when i use only message then ther is no problem but when the message have sub message like this
This throws the error.Any how the retag can support this??? |
@RajeshKumar1990 I fixed it. |
@qianlnk I am having a problem making this work.... does it support proto3?
Converting with: Result:
The db custom tag on id is missing.... |
@atamgp the first letter in field must be capitalized. |
@qianlnk thank you for the response. I changed the proto to: string ID = 1; // In above and below example the ` before db and at the end of line are omitted by github... but they are there. Unlike your example, only I am using a non existing tag like (no json or xml). ID string If you have another idea please let me know. I am looking into the plugin code right now. |
@atamgp have you install my retag plugin?
this is my try:
run:
and result: type ZZZ struct {
ID string ` protobuf:"bytes,1,opt,name=ID" json:"ID,omitempty" db:"id"`
ClientID string ` protobuf:"bytes,2,opt,name=ClientID" json:"ClientID,omitempty" db:"client_id"`
Datetime string ` protobuf:"bytes,3,opt,name=Datetime" json:"Datetime,omitempty" db:"datetime"`
} |
@qianlnk OK, I have it figured out a bit more :) message ZZZZ { Things I learned:
Working: When calling from within Z: protoc --go_out=plugins=grpc+retag:. zzz.proto not working: same commands which are working for 1 file but in order to process both proto files / Last: It is not clean to have all proto files and generated go files in the same folder. |
Has anyone found a solution for converting between type: string and type: bson.ObjectId as well? |
Please support this it is extremely useful and cuts down on needless code bloat. |
While this idea is super useful in Go, unfortunately no other language that proto supports can really make use of it. As such, changing/supporting this would be outside of the scope of the official Go protobuf package. |
@puellanivis, that is not true. Supporting custom tags would open protobufs to a plethora of possible tooling. |
I'm going to re-open this issue, but it does not mean that we're going to support this. I'd like to see more discussion on the feasibility of this. Some thoughts:
|
why not add a plugin like https://github.com/qianlnk/protobuf/tree/master/protoc-gen-go/retag This plugin is my original idea, don't take too much effort to attain it.But I believe that many people need it, so I hope more people can join in. |
@qianlnk Looked at your |
I am not contesting neild's post. How does the logic above apply to JSON support in the library? ;) Why JSON gets support and XML does not? Unsigned 64 bit in JSON is serialized as string. That's a very specific support for target format... We have to understand why people want this feature in the 1st place. The #1 motivation is to be able to tag structures for use by another library like DB layer (https://github.com/upper/db is just one example). IMO, Protocol Buffers team needs to focus on helping developers to make it easy to assign protocol buffer generated structures to types that have required tag data. Unfortunately, it is not possible today to do this:
The problem is with additional fields that PB generates. Those extra fields make it impossible to do an assignment of variables and requires manual field by field copy. I know that, because I had to code a lot of functions to copy data from PB and into PB. Specification is clear here: It would be great if PB team could help us with copy data graph operations. I think there are a number of approaches that can be taken here. |
While I understand the desire to do this, I don't see a way for this feature to be provided without going against protobuf's goal of being backwards and forwards compatible. In particular, the ability to add new fields without breaking current usages. In order for this assignment (or a cast) to work, it requires that the memory layout of |
We can go on and on with this feature, but if authors are very insistent on "my way or highway", there is not much we can do here. This is one of the most requested features desperately missed from the original proto-gen-go. Missed so much that people switched to gogoprotobuf just to have it. If implemented, it would allow proto-gen-go generated code to be useful in more than one scenario that original authors thought of, and just be a good "code neighbour" by allowing the feature that is not in direct use of the library, but potentially make life much easier to devs who intend to do "one more little thing" with generated code. Continuing with neighbour analogy, it also can choose to be an old sad grumpy "get off my lawn", and just never allow kids to draw on the sidewalk because "it is not what sidewalk is for". After 5 years of begging through this bug being open... This is actually quite sad. |
To my recollection we have never said "my way or highway". We acknowledge the benefit of this feature, but we have always pointed at technical reasons as to the detriments of this feature. Fundamentally, we prioritize adherence to the overall protobuf ecosystem and this feature goes against that priority. We understand that this is not the preference of some users, but engineering is about evaluating different options, understanding the benefits and detriments of all options and making a choice.
This statement makes it sound like we don't care about users. I should note that the fact that we spend significant amount of time engaging with people (even if we may disagree on technical points) should hopefully show that we care. |
I can go back starting from a very first response on this thread, and continue to comb this (or other) thread(s) to support of what I have said, but it is not a productive use of my or your time. You reopened this thread to gather users opinion? I think it is quite clear by now that we (users) wholeheartedly voted for it. I think there is quite an obvious way to "show that we care" other than continue to discuss it. Thank you for your time. |
Thank you @dsnet. You've done a great job supporting the project over the years. As a community we aim to provide feedback on real world implementations and sought-after features. I don't have much to add outside echoing @kulak where in most/all projects I've found myself copying data from/to PB. There is a natural flow into the DB layer and the Go ecosystem makes heavy use of tags within that layer. Capturing this in a single generated object would be a huge boost in productivity for the Go community. I may not agree with your final stand, but respect the adherence to the overall protobuf ecosystem. Keep up the great job. |
I understand the engineering decision, but it is nonetheless sad. Some projects don’t care about compatibility with other languages. And doing manual copying between structs is error prone, and boring... I’m not hopeful that someone will pickup and fix gogo, so that it becomes compatible with this library, at least not in a releasable form in the next five years... unless someone put a full time team on it for the next year or so... it will probably be easier to start from scratch. But I guess we can just hack this ourselves... I might do that when I get some spare time. Not likely to happen soon, so if anyone beats me to it, let me know. |
Candidly, we’ve concluded the cost of abandoning protos at the DB layer is lower than continuing to work with them due to lack of struct tags. I would summarize the maintainers’ stance as the Protobuf ecosystem is more important than the language ecosystem(s) that it works within. Given that most of us work primarily with language $x (in this case Go) more than we do with protos, this is petty sad. |
You should absolutely add language-specific features which are not accessible to other implementations if it means that language’s implementation conforms to the norms and expectations of that language. Every other Protobuf implementation does. Why is Go excluded?
Folks use Protobufs to define schemas used for other purposes than talking to a service. For example—WhatsApp uses it to serialize encrypted messages in a SQLite blob field. Square (and others) it to serialize complex data structures to MySQL.
You mean like this Java-specific feature? Regardless, your example (UTF-8 validation) is specious. Struct tags or the Go field names (see #555) only affect the Go implementation, and do not affect the interoperability of the protobuf messages. Do you plan to remove
Sure, struct tags can be used (or abused) for purposes outside of serialization. Just because there’s a footgun doesn’t mean folks are going to use it. Should you not trust your users, instead of trying to shield them from vague hypotheticals?
Hah, nope. |
I should have been clearer that I was referring to validation of
The The |
An option to select between |
@ydnar , we are about to face the same dilemma, and I am curious what did you choose as a replacement? |
We took the generated structs from protoc, deleted the protobuf-specific code, leaving struct tags as necessary. We already had a translation later between our db protos and API protos, so that layer didn’t need much additional work. |
I guess the underlying problem is that protobuf messages are considered to be DTOs, and not primary entities. Go walks quite a mile to have your primary entities to be used as DTOs as well -- one of its beauties. But I guess if one wants to use protobuf messages, it seems a DT layer is mandatory, as well as adding a translation of DTOs to primary entities and vice versa. One thing I can not wrap my head around, however, is that we are talking of protoc-gen-go: This is language specific, so it would not hurt other languages to have language specific features in the code generation -- after all, the according markup could be ignored in other languages by default. |
this is an old issue but I still see tags in pb.go files so maybe a sane approach at a .proto3 level could be to add a CRD generator (protoc-gen-crd plugin) and then allow to use it as a base for stuff like knative objects used by controllers or maybe php ORM yml custom schemas. |
I imagine there should be another issue for this discussion, and I am happy to open one, but this seemed like the place for the relevant discussion. That's especially true given the locking of #1142. I'm sort of stumped by the outcome of #1142 given the language from @neild regarding type control. I second the question in #52 (comment) though do understand that this is something of a slippery slope and may lead towards protocol buffer files which are too closely tied to an implementation.. What I'm really stuck on is nullability. I don't follow the principled argument against adding options to interact with the behavior of For what it's worth, the performance wins of avoiding allocations has lead cockroachdb/cockroach to use gogo/protobuf despite the fact that it is not maintained and will not obviously ever have support for the new library which is so much better. I suppose adding something like object pooling via something like what maybe is being discussed in #1192 could be helpful but tracking object lifetimes is quite hard. It feels like this has been a prolonged philosophical debate but I feel like the philosophy got lost w/ struct tags, which I can get behind. That argument however has extended to cover other more agnostic portions of the generated code. Value types are a major feature of go to provide not just access locality but to dramatically reduce the number of allocated objects making it more reasonable for performance sensitive code despite having a garbage collector. The lack of ability to utilize this key property of the language without forking the code generator feels confusing and at odds with a commitment to support go as a first-class user of protocol buffers. FWIW I do feel that controlling the type of the generated structs for messages is valuable but I don't have a clear vision on how to expose that |
I've submitted #1225 as a concrete and focused issue related to the above comment which was off-topic for this issue. |
I have written a tool named protoc-gen-go-tag to this feature, custom struct tags for protobuf!
|
So, is there any news about this customization ability for protoc-gen? |
tested https://github.com/srikrsna/protoc-gen-gotag version: v1beta1
plugins:
- name: go
out: .
opt: paths=source_relative
- name: gotag
out: .
opt: paths=source_relative
- name: go-grpc
out: .
opt: paths=source_relative,require_unimplemented_servers=true
- name: validate
out: .
opt: lang=go,paths=source_relative |
This comment has been minimized.
This comment has been minimized.
Need this feature,A lot of request and response are not standard json lowercase ,so we need this feature to custom json tag. |
Is it possible to define a
message
with custom tags? For example defining a json name that isn't just the lowercase field name or adding a bson tag.The text was updated successfully, but these errors were encountered: