feat!: add new sync proto, rm old sync proto #99
Conversation
Signed-off-by: Todd Baert <[email protected]>
toddbaert
requested review from
beeme1mr,
thisthat,
bacherfl and
Kavindu-Dodan
| @@ -131,3 +130,14 @@ func (s *SyncImpl) FetchAllFlags(context.Context, *v1.FetchAllFlagsRequest) (*v1 | |||
| FlagConfiguration: string(marshalled), | |||
| }, nil | |||
| } | |||
|
|
|||
| func (s *SyncImpl) GetMetadata(context.Context, *v1.GetMetadataRequest) (*v1.GetMetadataResponse, error) { | |||
There was a problem hiding this comment.
We need to implement this now.
| { | ||
| Key: "paths", | ||
| Value: fmt.Sprintf("%v", s.fw.paths), | ||
| }, |
There was a problem hiding this comment.
I wanted to add some kind of meaningful metadata, even though this is really just a test harness. I chose to send back all the watched flags:
{
"metadata": [
{
"key": "paths",
"value": "[//home/todd/git/flagd-testbed/flags/testing-flags.json //home/todd/git/flagd-testbed/flags/custom-ops.json]"
}
]
}
There was a problem hiding this comment.
Looking at this more, I wonder if we should have used a map here.
This key/value construct used to be the recommended way, but maps exist now: https://protobuf.dev/programming-guides/proto3/#maps
Interestingly, the backwards compatibility section here seems to imply we could make this change in a non-breaking way because @bacherfl followed the convention exactly.
There was a problem hiding this comment.
Good point. Per details in the backward compatibility section, I assume the map is some syntactic sugar for key-value message definitions. And it should make schema easy to understand.
If this is not a breaking change (per buf validations) I think it's fine to upgrade to map<string, string>.
| @@ -92,7 +93,6 @@ func (s *SyncImpl) SyncFlags(req *v1.SyncFlagsRequest, stream syncv1grpc.FlagSyn | |||
| // initial read | |||
| err := stream.Send(&v1.SyncFlagsResponse{ | |||
| FlagConfiguration: string(s.fw.getCurrentData()), | |||
| State: v1.SyncState_SYNC_STATE_ALL, | |||
| "buf.build/gen/go/open-feature/flagd/grpc/go/flagd/sync/v1/syncv1grpc" | ||
| v1 "buf.build/gen/go/open-feature/flagd/protocolbuffers/go/flagd/sync/v1" |
There was a problem hiding this comment.
This is all it takes to use the new proto.
toddbaert
changed the title
flagd.sync.v1sync.v1(breaking)metadata