[
https://issues.apache.org/jira/browse/HDDS-6365?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-6365:
-----------------------------
Description:
h2. Motivation
The motivation behind this is that currently in the master branch we use
{{kerberosID}} as the protobuf message field name while it really means
{{accessId}}. For example here:
https://github.com/apache/ozone/blob/master/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto#L1323
It just so happens to be the case that before Multi-Tenancy (HDDS-4944) is
implemented, kerberosID is equivalent to accessId in the context of Ozone S3
access. But with Multi-Tenancy feature this is no longer the case -- accessId
does not equal to kerberosID any more.
And it could be confusing for other developers to work on S3 / Multi-Tenancy
related stuff in the future. e.g. those {{getKerberosID}} calls really means
{{getAccessId}}, and {{setKerberosID}} really is {{setAccessId}}.
h2. Status Quo
Right now {{proto-backwards-compatibility}} uses default
[protolock|https://github.com/nilslice/protolock#usage] command line argument,
effectively enforcing strict mode for the
[rules|https://github.com/nilslice/protolock#rules-enforced], which doesn't
allow changing existing protobuf field names:
{code:xml|title=https://github.com/apache/ozone/blob/68c5ac5df4fbc0edd7114394112fa696a3dc9229/pom.xml#L1619-L1633}
<groupId>com.salesforce.servicelibs</groupId>
<artifactId>proto-backwards-compatibility</artifactId>
<version>${proto-backwards-compatibility.version}</version>
<configuration>
<protoSourceRoot>${basedir}/target/classes</protoSourceRoot>
</configuration>
{code}
Changing the field name itself does not break wire compatiblity (field type and
field number are still the same), unless the protobuf message is decoded into
JSON at some point (which could use the field name as key). Ref:
https://stackoverflow.com/a/45431953
h2. Proposal
Add {{<options>--strict false</options>}} in {{pom.xml}} so that strict mode is
off, which as a side affect also turns off two other rules currently enforced
according to [protolock
readme|https://github.com/nilslice/protolock#rules-enforced]. I'm not sure if
protolock provides a way for more granular control of which exact rule to turn
off.
{code}
No Removing Reserved Fields
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any reserved field has been removed.
Note: This rule is not enforced when strict mode is disabled.
{code}
{code}
No Changing Field Names
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any message's previous fields have been renamed.
Note: This rule is not enforced when strict mode is disabled.
{code}
{code}
No Removing RPCs
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any RPCs provided by a Service have been removed.
Note: This rule is not enforced when strict mode is disabled.
{code}
was:
Right now {{proto-backwards-compatibility}} uses default
[protolock|https://github.com/nilslice/protolock#usage] command line argument,
effectively enforcing strict mode for the
[rules|https://github.com/nilslice/protolock#rules-enforced], which doesn't
allow changing existing protobuf field names:
{code:xml|title=https://github.com/apache/ozone/blob/68c5ac5df4fbc0edd7114394112fa696a3dc9229/pom.xml#L1619-L1633}
<groupId>com.salesforce.servicelibs</groupId>
<artifactId>proto-backwards-compatibility</artifactId>
<version>${proto-backwards-compatibility.version}</version>
<configuration>
<protoSourceRoot>${basedir}/target/classes</protoSourceRoot>
</configuration>
{code}
Changing the field name itself does not break wire compatiblity (field type and
field number are still the same), unless the protobuf message is decoded into
JSON at some point (which could use the field name as key). Ref:
https://stackoverflow.com/a/45431953
Proposal:
Add {{<options>--strict false</options>}} in {{pom.xml}} so that strict mode is
off, which as a side affect also turns off two other rules currently enforced
according to [protolock
readme|https://github.com/nilslice/protolock#rules-enforced]. I'm not sure if
protolock provides a way for more granular control of which exact rule to turn
off.
{code}
No Removing Reserved Fields
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any reserved field has been removed.
Note: This rule is not enforced when strict mode is disabled.
{code}
{code}
No Changing Field Names
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any message's previous fields have been renamed.
Note: This rule is not enforced when strict mode is disabled.
{code}
{code}
No Removing RPCs
Compares the current vs. updated Protolock definitions and will return a list
of warnings if any RPCs provided by a Service have been removed.
Note: This rule is not enforced when strict mode is disabled.
{code}
> Relax protolock rule so changing field names are allowed
> --------------------------------------------------------
>
> Key: HDDS-6365
> URL: https://issues.apache.org/jira/browse/HDDS-6365
> Project: Apache Ozone
> Issue Type: Improvement
> Components: build
> Reporter: Siyao Meng
> Priority: Major
>
> h2. Motivation
> The motivation behind this is that currently in the master branch we use
> {{kerberosID}} as the protobuf message field name while it really means
> {{accessId}}. For example here:
> https://github.com/apache/ozone/blob/master/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto#L1323
> It just so happens to be the case that before Multi-Tenancy (HDDS-4944) is
> implemented, kerberosID is equivalent to accessId in the context of Ozone S3
> access. But with Multi-Tenancy feature this is no longer the case -- accessId
> does not equal to kerberosID any more.
> And it could be confusing for other developers to work on S3 / Multi-Tenancy
> related stuff in the future. e.g. those {{getKerberosID}} calls really means
> {{getAccessId}}, and {{setKerberosID}} really is {{setAccessId}}.
> h2. Status Quo
> Right now {{proto-backwards-compatibility}} uses default
> [protolock|https://github.com/nilslice/protolock#usage] command line
> argument, effectively enforcing strict mode for the
> [rules|https://github.com/nilslice/protolock#rules-enforced], which doesn't
> allow changing existing protobuf field names:
> {code:xml|title=https://github.com/apache/ozone/blob/68c5ac5df4fbc0edd7114394112fa696a3dc9229/pom.xml#L1619-L1633}
> <groupId>com.salesforce.servicelibs</groupId>
> <artifactId>proto-backwards-compatibility</artifactId>
> <version>${proto-backwards-compatibility.version}</version>
> <configuration>
> <protoSourceRoot>${basedir}/target/classes</protoSourceRoot>
> </configuration>
> {code}
> Changing the field name itself does not break wire compatiblity (field type
> and field number are still the same), unless the protobuf message is decoded
> into JSON at some point (which could use the field name as key). Ref:
> https://stackoverflow.com/a/45431953
> h2. Proposal
> Add {{<options>--strict false</options>}} in {{pom.xml}} so that strict mode
> is off, which as a side affect also turns off two other rules currently
> enforced according to [protolock
> readme|https://github.com/nilslice/protolock#rules-enforced]. I'm not sure if
> protolock provides a way for more granular control of which exact rule to
> turn off.
> {code}
> No Removing Reserved Fields
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any reserved field has been removed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
> {code}
> No Changing Field Names
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any message's previous fields have been renamed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
> {code}
> No Removing RPCs
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any RPCs provided by a Service have been removed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
