-
Notifications
You must be signed in to change notification settings - Fork 5
feat: OpenAPI proposal for V1 OFREP #2
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
Conversation
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
thomaspoignant
left a comment
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 like this interface that is super simple for flag evaluation.
I am just wondering if we don't need extra information in the success schema to help the provider to deal with the result of this flag.
It may be too early, but I am thinking about extra information such as:
- provider can cache the value
- or tracking is activated on this flag
- maybe a version of the flag (even if this could be in the metadata).
nicklasl
left a comment
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.
Thanks a lot for kick starting this!
I left a bunch of comments mostly from the static paradigm context :)
|
Hey thanks for kicking it off. |
From the survey results it seems that polling is probably a better option here. The provider can probably support something like:
But to start small and based on the survey results I think we should start with polling 1st. |
I also think we can first focus on the dynamic context paradigm and then focus on the dynamic context paradigm. But this is something we can collectively decide. So far I have the following key points for the next session (from a technical stand point)
|
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
|
After discussing with @Kavindu-Dodan I've modified the openAPI spec to include some changes, particularly related to the static context paradigm and polling.
I've kept the explicit type on cc @Kavindu-Dodan @jonathannorris @nicklasl @lukas-reining @beeme1mr @lukas-reining |
When the key is in the path, it can involve needing for url encode it for special chars. It can create unexpected behavior in some web server configurations. To overcome it from beginning, what do you think of passing the flag key on the request payload as some Json? It can also make it flexible for adding additional params if needed. |
I understand your concern here, but having the key in the path is more RESTful, adding it and encoding it, is a classic pattern on how to build APIs so I don't see that as a blocker for the flag management systems. |
|
I think we should also specify that we expect optional |
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com> Signed-off-by: Thomas Poignant <thomas.poignant@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
…/openapi-proposal
Signed-off-by: Lukas Reining <lukas.reining@codecentric.de>
Signed-off-by: Lukas Reining <lukas.reining@codecentric.de>
ADRsAs discussed in the OFREP meeting and with @Kavindu-Dodan and @thomaspoignant I added ADRs for the decisions that we felt like are important to track and have a record of. We came up wit the following points to track in an adr:
The structure of ADRs is bases on Michael Nygards template: https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions |
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
|
Nice work @Kavindu-Dodan and everyone who helped! I'm excited about this. 🚀 |
Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
nicklasl
left a comment
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.
🚀 Good stuff! Love the ADRs!
dyladan
left a comment
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.
Looks good to me. I like having the ADRs as well
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
|
Update 05.03.2024 Based on feedback, I have done following improvements,
|
|
If there are no more concerns, I will merge this version of the OpenAPI specification on 07.03.2024. Once merged, we could continue to improve the API through issues and follow-up PRs. I expect such changes to be non-breaking so that OFREP adaptions can proceed smoothly. |
OFREP evaluation APIs
A proposal for the initial version of the OpenAPI specification for the OFREP service.
Please feel free to review, suggest, and propose changes so we can collectively define the service.
Why OpenAPI?
According to the survey results, JSON over HTTP was the most widely used technology by vendors.
What's in this PR?
This PR mainly focuses on the flag evaluation. It contains,
/ofrep/v1/evaluate/{key}:This endpoint evaluates an individual feature flag from the flag management system
/ofrep/v1/evaluateThis endpoint mainly focuses on the static context paradigm and allows multiple flags (bulk) evaluations based on context, authorization token, or flag keys in the request payload.
This PR does NOT focus on telemetry or flag evaluation metrics. This will be a follow-up discussion.