> According to https://protobuf.dev/programming-guides/proto2/#enum , we should add [deprecated=true] to each field instead of removing the old protos. Please try.
It seems in the current proto2 version we are using deprecated fields are not being supported in enums. Also the reserved keyword that is used to reserve field IDs is not supported either in proto2. This was found while trying to remove the listTrash and recoverTrash APIs as a part of HDDS-11251 On Mon, 19 Aug 2024 at 22:05, Tsz Wo Sze <szets...@gmail.com> wrote: > > Will there be any compatibility issues after moving enum > `StorageTypeProto` from `ScmServerDatanodeHeartbeatProtocol.proto` to > `hdds.proto` and standardize, and change all the places where > `StorageTypeProto` is used in the existing protobuf to > `hdds.proto#StorageTypeProto`? > > There are different levels of compatibility: > > For binary compatibility, it is okay since > "enum is compatible with int32, uint32, int64, and uint64 in terms of wire > format" > according to https://protobuf.dev/programming-guides/proto2/#updating > It is easy to test: write both the new and the old protos to files and then > compare the binaries. > > For API compatibility, the Java classes will be different. The question: > are we using it in a public API? It also seems not since we are using > - org.apache.hadoop.hdds.protocol.StorageType > > in the public APIs. (e.g. OzoneBucket.getStorageType(). ) > > According to https://protobuf.dev/programming-guides/proto2/#enum , we > should add [deprecated=true] to each field instead of removing the old > protos. Please try. > > BTW, we also have another StorageTypeProto defined > in OmClientProtocol.proto. We should deprecate it as well. > > Please file a JIRA for the further discussion. > > Tsz-Wo > > > > > On Mon, Aug 19, 2024 at 2:17 AM mrchenx <mrch...@126.com> wrote: > > > Dear Ozone Devs, > > > > > > > > > > I am currently developing some features related to StoragePolicy and have > > encountered an issue for which I am seeking a solution. > > > > > > > > > > ## What I want to do > > > > - Move the enum `StorageTypeProto` from > > `ScmServerDatanodeHeartbeatProtocol.proto` to `hdds.proto`. > > > > - Most of the protobuf definitions that need to be imported in our > > project are located in `hdds.proto`, so I want to move enum > > `StorageTypeProto` to `hdds.proto`. > > > > > > > > > > - Standardize the usage of `hdds.proto#StorageTypeProto` across all > > protobuf files in the project. > > > > - This would require removing > > `ScmServerDatanodeHeartbeatProtocol.proto#StorageTypeProto` and > > `OmClientProtocol.proto#StorageTypeProto`. > > > > > > > > > > ## The error I encountered > > > > However, I encountered an issue where `proto-backwards-compatibility` > > throws errors after moving the `enum StorageTypeProto`. > > > > > > > > > > ```bash > > > > [INFO] --- > > proto-backwards-compatibility:1.0.7:backwards-compatibility-check > (default) > > @ hdds-interface-server --- > > > > //... > > > > [ERROR] CONFLICT: "StorageTypeProto" field: "ARCHIVE" has been removed, > > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" field: "DISK" has been removed, but > > is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" field: "PROVIDED" has been removed, > > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" field: "RAM_DISK" has been removed, > > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" field: "SSD" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" integer: "1" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" integer: "2" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" integer: "3" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" integer: "4" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > [ERROR] CONFLICT: "StorageTypeProto" integer: "5" has been removed, but > is > > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > > > ``` > > > > > > > > > > ## My question > > > > - Will there be any compatibility issues after moving enum > > `StorageTypeProto` from `ScmServerDatanodeHeartbeatProtocol.proto` to > > `hdds.proto` and standardize, and change all the places where > > `StorageTypeProto` is used in the existing protobuf to > > `hdds.proto#StorageTypeProto`? > > > > - According to my tests, when I add the compile flag > > `-DallowBreakingChanges=true` to bypass the errors, old clients can still > > successfully execute create/list bucket operations. > > > > - As I understand it, the `field id` of a protobuf enum is what > matters, > > as the `id` is the valid payload, while changes to the enum's field names > > or the package in which the enum is defined should not cause > compatibility > > issues (though we will need to update the relevant Java code to reflect > > these changes). > > > > > > > > > > - If this does not cause compatibility issues, how should I handle the > > errors reported by `proto-backwards-compatibility`? > > > > > > >
