feat: return flag metadata as well #1476
Conversation
dosubot
bot
added
the
size:M
✅ Deploy Preview for polite-licorice-3db33c ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
aasifkhan7
changed the title
aasifkhan7
force-pushed
the
feat/flag-metadata
branch
3 times, most recently
from
3b6e501 to
c55537f
Compare
Signed-off-by: Aasif Khan <[email protected]>
aasifkhan7
force-pushed
the
feat/flag-metadata
branch
from
c55537f to
931c120
Compare
Signed-off-by: Aasif Khan <[email protected]>
|
Hey @aasifkhan7, I tested this locally and wasn't able to see flag metadata returned as a response. Here's the flag configuration I was using: {
"flags": {
"myBoolFlag": {
"state": "ENABLED",
"variants": {
"on": true,
"off": false
},
"defaultVariant": "on"
}
},
"metadata": {
"id": "test",
"version": "1"
}
}Here are the test evaluations I tried: curl -X POST "localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean" -d '{"flagKey":"myBoolFlag","context":{}}' -H "Content-Type: application/json"and curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags'Please map the |
Signed-off-by: Aasif Khan <[email protected]>
aasifkhan7
force-pushed
the
feat/flag-metadata
branch
from
f75d5b5 to
dd48ee3
Compare
|
Hi @beeme1mr, I've updated the commit to include the setting of |
There was a problem hiding this comment.
@aasifkhan7 nice job so far!
OFREP works as expected:
curl -X POST 'http://localhost:8016/ofrep/v1/evaluate/flags' | jq
{
"flags": [
{
"value": 1,
"key": "myIntFlag",
"reason": "STATIC",
"variant": "one",
"metadata": {
"flagSetId": "test",
"flagSetVersion": "1"
},
...
}
]
}curl -X POST "localhost:8013/flagd.evaluation.v1.Service/ResolveBoolean" -d '{"flagKey":"myBoolFlag","context":{}}' -H "Content-Type: application/json" | jq
{
"value": true,
"reason": "STATIC",
"variant": "on",
"metadata": {
"flagSetId": "test",
"flagSetVersion": "1"
}
}I left some comments - I also think we need some basic test coverage for this, but nice job!
core/pkg/evaluator/json.go
Outdated
| if flagMetadata.ID != "" { | ||
| metadata[ID] = flagMetadata.ID | ||
| } | ||
| if flagMetadata.Version != "" { | ||
| metadata[VERSION] = flagMetadata.Version | ||
| } |
There was a problem hiding this comment.
I'm not sure what the value of these is, since we are not returning them in any responses. Maybe I'm confused.
There was a problem hiding this comment.
@toddbaert The idea behind this is that each flag could also have its own specific metadata. For example, a flag configuration might look like this:
{
"flags": {
"myBoolFlag": {
"state": "ENABLED",
"variants": {
"on": true,
"off": false
},
"defaultVariant": "on",
"metadata": {
"id": "sso/dev",
"version": "1.0.0"
}
}
},
"metadata": {
"id": "test",
"version": "1"
}
}
Here, the flag-level metadata (id, version) allows for more granular identification and versioning of individual flags, in addition to the overarching metadata. Let me know your thoughts!
toddbaert
requested review from
hairyhenderson,
Kavindu-Dodan,
aepfli,
toddbaert,
warber,
craigpastro,
gruebel and
thisthat
Signed-off-by: Aasif Khan <[email protected]>
Signed-off-by: Aasif Khan <[email protected]>
|
👋 Hello folks, |
Signed-off-by: Aasif Khan <[email protected]>
@apodgorbunschih I've updated the PR to handle any fields that come in metadata. Please re-review @beeme1mr @toddbaert |
|
I've also addressed the unrelated license compliance issue. |
Signed-off-by: Aasif Khan <[email protected]>
| for key, flag := range newFlags.Flags { | ||
| if flag.Metadata == nil { | ||
| flag.Metadata = make(map[string]interface{}) | ||
| } | ||
| for metaKey, metaValue := range configData.Metadata { | ||
| flag.Metadata[metaKey] = metaValue | ||
| } | ||
| newFlags.Flags[key] = flag | ||
| } |
There was a problem hiding this comment.
configData.Metadata should have a lower merge priority than flag.Metadata.
This PR
Related Issues
Fixes #1464
Notes
This PR ensures that flag metadata is included and handled correctly in responses.