-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: return flag metadata as well #1476
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for polite-licorice-3db33c ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3b6e501
to
c55537f
Compare
Signed-off-by: Aasif Khan <aasifk106@gmail.com>
c55537f
to
931c120
Compare
Signed-off-by: Aasif Khan <aasifk106@gmail.com>
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 <aasifk106@gmail.com>
f75d5b5
to
dd48ee3
Compare
Hi @beeme1mr, I've updated the commit to include the setting of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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!
Signed-off-by: Aasif Khan <aasifk106@gmail.com>
This PR
Related Issues
Fixes #1464
Notes
This PR ensures that flag metadata is included and handled correctly in responses.