Some adjustments to protobuf field and request names #547
Conversation
gbregman
force-pushed
the
devel
branch
2 times, most recently
from
f26429d
to
4d016af
Compare
|
@epuertat notice that I didn't rename get_subsystems() and set_ana_state() to have the verb in the end as they are used in the Ceph monitor. We might do it later before merging the monitor. |
gbregman
force-pushed
the
devel
branch
2 times, most recently
from
da123ce
to
308df27
Compare
gbregman
force-pushed
the
devel
branch
2 times, most recently
from
7e4e163
to
05f42f1
Compare
gbregman
force-pushed
the
devel
branch
2 times, most recently
from
9ba46c4
to
e3e42f0
Compare
There was a problem hiding this comment.
CC: @nizamial09
At this point in the release cycle, I'm wondering if we should go for this or wait for a 2.y.z release (and even though, we should aim at minimizing the amount of breaking changes, and keeping as much as possible backward-compatible interfaces).
As is, this PR currently brings breaking changes and would need updates in the Ceph side (Dashboard embeds a .proto file).
The alternative of making this backward compatible is viable though, and involves leaving both interfaces. Something along the lines of:
service Gateway {
rpc list_namespaces(list_namespaces_req) returns(namespaces_info) {}
message list_namespaces_req {
string subsystem = 1;
optional uint32 nsid = 2;
optional string uuid = 3;
}
...
}
service GatewayV2 {
rpc namespace_list(namespace_list_req) returns(namespaces_info) {}
message namespace_list_req {
string subsystem_nqn = 1;
optional uint32 nsid = 2;
optional string uuid = 3;
}
...
}What do you think @gbregman ?
|
@epuertat we plan to advance the version in the near future. I'm not sure having a double interface is worth it. I would rather to either leave it as it is now or change it now before we release. |
I understand that after the ceph-mon based HA, ceph-nvmeof and ceph projects are no longer loosely coupled, and versioning looks granted 'cause everything is deployed together. Quite the opposite, Ceph Dashboard is committed to maintain a stable RESTful API across major Ceph releases (that including clients outside the Ceph ecosystem), and that can be unattainable if the underlying gRPC API is arbitrarily mutating (not to mention the complexities of version upgrades). So, before making any post-v1 breaking-changes to the gRPC proto, please let's sit all together and discuss about versioning & stability approaches (CC: @caroav @oritwas @jdurgin ). I'm sure that you'll greatly appreciate it when a new Ceph release is deployed and you don't have to worry about upgrade/downgrade processes. |
|
@gbregman if backward compatibility is not considered for this PR, then I'm ok with closing it. As said, thank you very much for the effort you put here! ❤️ |
|
@epuertat if these changes require work that we cannot afford right now in the API, then we will not merge it now. I wanted to have a clean first version. But its up to you. We can always do it in V2. |
|
BTW, it'd be great if we would increase the GW version digits with API changes. That would make it easy to "name" at what stage every component is (e.g.: Ceph Dashboard technically supports some My suggestion is that, at least during intra-release changes, we still increase some version digit (e.g.: according to Semantic Versioning: |
|
@epuertat so you suggest that the GW version will track only API changes, or also code fixes? Do we need one version to API and another one to general code fixes/releases maybe? |
|
PR was closed for now. We'll consider to re-do it in a future version. |
@caroav: API changes is the top reason to include versioning, but not only API changes. Quoting Semantic Versioning spec:
I understand that within pre-release development, versioning might be relaxed, but here we have at least 3 components from different codebases (Ceph NVMe-oF Gateway, ceph-mon and Ceph-Dashboard) that need to be in sync. If versioning is not respected even within pre-release period, it basically shifts all the burden to the consumers of the NVMe-oF API (ceph-mon and Ceph-Dashboard). NVMe-oF team is taking care of keeping ceph-mon in sync, but not the Dashboard, so honoring an API versioning is the best way to coordinate work across components (and detect potential version mismatches). |
Fixes #518