Description:
Ballerina grammar doesn't support defining annotation for struct fields like below,
bal
struct HelloResponse {
@grpc:protobuf(fieldType="string”,index=1,name=”message")
string message;
}
When we are defining custom types using structs, we may need to define additional properties for the field.
e.g: when we define grpc service in ballerina, we need to define field index in message type. If Ballerina supports field annotation, we can define field index as an annotation.
Steps to reproduce:
This is an improvement.
Affected Versions:
N/A
OS, DB, other environment details and versions:
Related Issues (optional):
Suggested Labels (optional):
improvement
Suggested Assignees (optional):
@hasithaa
I just ran into this, and I would qualify this as higher precedence than improvement. This is a compatibility-breaking feature.
For example:
I created a record with 4 fields, which then generates a protobuf file. Another team in my org is now using that protobuf file to build their service. Both our services go to production.
Now, I add a field to my record but I add it _above_ the other fields. This will actually _renumber_ the protobuf field indexes below the newly added field. This will _break_ all clients. It's far too easy for someone to accidentally break compatibility without knowing it because of the implicit magic happening from the record structure to protobuf conversion.
This also goes to adoption. You're not going to get people to buy into your automagic code-first protobuf generation if it's this easy to accidentally break the contract between service and client.
@autodidaddict Totally agreed with your point. We are also looking to support contract first approach where we define the proto contract first and generate service skeleton. So that we can support backward compatibility. I started discussion [1] in google group. and created separate ticket #8954 to address it.
@daneshk @wggihan @pamod
Do we need to support this for both records and objects. ? Either case, this is deviating from Ballerina-Language-Specification-WD-2018-07-12. Shall we discuss this in the dev group?
Related Spec issue: https://github.com/ballerina-platform/ballerina-spec/issues/299
This was fixed in #21529. Hence closing this.
Most helpful comment
@daneshk @wggihan @pamod
Do we need to support this for both records and objects. ? Either case, this is deviating from Ballerina-Language-Specification-WD-2018-07-12. Shall we discuss this in the dev group?