Move Message proto to types #9742
Conversation
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
| @@ -55,17 +54,9 @@ message PublishRequest { | |||
| } | |||
|
|
|||
| message ForwardRequest { | |||
| Envelope envelope = 1; | |||
| containerd.types.Envelope envelope = 1; | |||
There was a problem hiding this comment.
Just to confirm, since the structure is the same, the bytes on the wire should be the same. The protobuf type name changing won't have any impact on deserialization between v1 and v2?
There was a problem hiding this comment.
Message names aren't typically sent on the network (Any is an exception, but we don't use it here), protobuf cares about message fields types/numbers to be compatible.
I googled a few confirmations just to be sure:
| option go_package = "github.com/containerd/containerd/v2/api/types;types"; | ||
|
|
||
| message Envelope { | ||
| option (containerd.types.fieldpath) = true; |
There was a problem hiding this comment.
Do we not need the go-fieldpath generator for this?
There was a problem hiding this comment.
Doesn't look like it's used anywhere in the codebase.
Though the old versions had the field code generated.
|
If possible, it should have upgrade test for this. I don't worry about CRI. just want to confirm there is no regression for docker engine. |
We currently duplicate
Envelopeproto in several places and I need another one for #9704.This PR moves
Envelopestruct totypes/, so we can reuse it.Moving proto messages expected to be backward compatible as long as we preserve memory layout (which we do).