feat: add copies of existing evaluation and sync schemas with adapted names #109
Conversation
… names Signed-off-by: Florian Bacher <[email protected]>
bacherfl
force-pushed
the
new-schema-names
branch
from
cf6135e to
023bdc1
Compare
|
@toddbaert this should be ready for review now. There is obviously some duplication involved, but in my opinion it is manageable and the core logic can be reused to a large extend. |
Agreed. Especially because much of the duplication will be deleted pre flagd 1.0, I'm fine with this duplication. We might want to extract some duplication into different files and add more comments, but the PoC proves its point here! Thanks @bacherfl Something else to note is the flagd-proxy / core uses the sync service as well. I think we might to prevent any breakage, we'd need to also serve the new sync-service there too. I think this will be easier, since it's only pure gRPC, not the connect protocol... but you may want to add this to your PoC to validate it. cc @Kavindu-Dodan on this one. |
| option go_package = "flagd/evaluation/v1"; | ||
| option java_package = "dev.openfeature.flagd.grpc.evaluation"; | ||
| option php_namespace = "OpenFeature\\Providers\\Flagd\\Schema\\Grpc\\Evaluation"; | ||
| option ruby_package = "OpenFeature::FlagD::Provider::Grpc::Evaluation"; |
There was a problem hiding this comment.
@toddbaert should we use this as an opportunity to lowercase the D based on the updated naming guidelines?
… client for the sync api Signed-off-by: Florian Bacher <[email protected]>
… client for the sync api Signed-off-by: Florian Bacher <[email protected]>
Alright, I will also have a look at the flagd-proxy regarding this, but I agree that this will likely not cause any problems as well - Will keep you updated on this |
Signed-off-by: Florian Bacher <[email protected]>
|
I'm ready to approve once we have some consensus on this question. |
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
There was a problem hiding this comment.
LGTM but very interested to hear from @Kavindu-Dodan and @beeme1mr
🤖 I have created a release *beep* *boop* --- <details><summary>protobuf: 0.5.2</summary> ## [0.5.2](protobuf-v0.5.1...protobuf-v0.5.2) (2023-10-24) ### ✨ New Features * add better-named evaluation/sync protos ([#109](#109)) ([7b8e75d](7b8e75d)) ### 🧹 Chore * fixed typo in the flag evaluation api comment ([c6cc820](c6cc820)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Closes #948
This PR introduces new names for the sync and evaluation schemas. The old schemas are kept for now, to support backwards compatibility.
I will leave this one in draft mode for now, as I would like to work on a PoC on how to best support this in flagd with as little duplication as possible.