Skip to content

feat: make flags property mandatory for bulk evaluation success response #27

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.

Sign up for GitHub

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

Jump to bottom
Merged
Kavindu-Dodan merged 1 commit into from
Jun 25, 2024
Merged

feat: make flags property mandatory for bulk evaluation success response #27

Kavindu-Dodan merged 1 commit into from
Jun 25, 2024

Conversation

@Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Jun 25, 2024 •
edited
Loading

Background

bulkEvaluationSuccess right now only defines flags property. This is the main property the response is expected to contain.

Proposal

With this PR, I have made the flags property mandatory in the bulk evaluation response. This confirms to the current JavaScript implementation [1] where flags is required to make the response a valid one.

[1] - https://github.com/open-feature/js-sdk-contrib/blob/ofrep-core-v0.1.5/libs/shared/ofrep-core/src/lib/model/bulk-evaluation.ts#L32-L38

@Kavindu-Dodan Kavindu-Dodan requested a review from a team as a code owner June 25, 2024 18:53
lukas-reining
lukas-reining approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an important one! :)

@Kavindu-Dodan Kavindu-Dodan merged commit 18db9b2 into main Jun 25, 2024
@Kavindu-Dodan Kavindu-Dodan deleted the chore/make-flags-mandatory branch June 25, 2024 19:05
@Kavindu-Dodan Kavindu-Dodan mentioned this pull request Jun 25, 2024
Merged
toddbaert pushed a commit to open-feature/flagd that referenced this pull request Jun 27, 2024
@Kavindu-Dodan
feat: add mandatory flags property in bulk response (#1339)
b20266e 
This PR improves flagd bulk evaluation empty response by adding required
`flags` property.

Related - open-feature/protocol#27

Signed-off-by: Kavindu Dodanduwa <[email protected]>
thomaspoignant pushed a commit that referenced this pull request May 12, 2025
@Kavindu-Dodan @thomaspoignant
make flags property mandatory for bulk evaluation success response (#27)
2d4a767 
Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
thomaspoignant pushed a commit that referenced this pull request May 16, 2025
@Kavindu-Dodan @thomaspoignant
make flags property mandatory for bulk evaluation success response (#27)
87cbb1a 
Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
thomaspoignant added a commit that referenced this pull request May 16, 2025
@thomaspoignant @markphelps
@beeme1mr @Kavindu-Dodan @renovate @lukas-reining @grimly @erka @toddbaert @czechboy0
feat: Client provider guidelines (#14)
66f2683 
* feat: Client provider spec

Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Mark Phelps <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update provider/specs/client.md

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* update with review comments

Signed-off-by: Thomas Poignant <[email protected]>

* Move to guideline folder

Signed-off-by: Thomas Poignant <[email protected]>

* Replace specification

Signed-off-by: Thomas Poignant <[email protected]>

* add OpenAPI spec validator based on redocly cli (#15)

Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* chore(deps): update actions/checkout action to v4 (#16)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Thomas Poignant <[email protected]>

* Update guideline/static-context-provider.md

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update guideline/static-context-provider.md

Co-authored-by: Lukas Reining <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update guideline/static-context-provider.md

Co-authored-by: Lukas Reining <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* update guidelines after removing configuration endpoint

Signed-off-by: Thomas Poignant <[email protected]>

* adding change context

Signed-off-by: Thomas Poignant <[email protected]>

* feat: Allow any reason (#20)

Signed-off-by: Thomas Poignant <[email protected]>

* feat!: change minPollingInterval field name to mention millisecond (#25)

* feat!: change minPollingInterval field name to mention millisecond

Signed-off-by: Thomas Poignant <[email protected]>

* change name to ms

Signed-off-by: Thomas Poignant <[email protected]>

---------

Signed-off-by: Thomas Poignant <[email protected]>

* feat: Group API in core and extensions (#23)

Signed-off-by: Thomas Poignant <[email protected]>

* feat!: Reverse the logic for supportedTypes (#24)

* feat!: Reverse the logic for supportedTypes

Signed-off-by: Thomas Poignant <[email protected]>

* fix

Signed-off-by: Thomas Poignant <[email protected]>

---------

Signed-off-by: Thomas Poignant <[email protected]>

* doc: adding providers link (#26)

Signed-off-by: Thomas Poignant <[email protected]>

* make flags property mandatory for bulk evaluation success response (#27)

Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* feat: Typo in header name (#28)

Signed-off-by: Thomas Poignant <[email protected]>

* add optional targeting key property (#30)

Signed-off-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* fix: use correct header name for 429 bulk response (#32)

Signed-off-by: Roman Dmytrenko <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* feat: Specify caching for OFREP in server providers (#17)

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Kavindu Dodanduwa <[email protected]>
Co-authored-by: Kavindu Dodanduwa <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* feat: add flag set metadata for bulk response and failures (#34)

Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Fixups: Add operationIds, remove invalid property, fix tag casing (#35)

Signed-off-by: Honza Dvorsky <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* ci: switch OpenAPI validators (#36)

Signed-off-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* fix: address style issues

Signed-off-by: Thomas Poignant <[email protected]>

* Update static-context-provider.md

Co-authored-by: Lukas Reining <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update static-context-provider.md

Co-authored-by: Lukas Reining <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* adding timeout

Signed-off-by: Thomas Poignant <[email protected]>

* feat: Server provider guidelines (#42)

* feat: Server provider guideline

Signed-off-by: Thomas Poignant <[email protected]>

* adding timeout

Signed-off-by: Thomas Poignant <[email protected]>

---------

Signed-off-by: Thomas Poignant <[email protected]>

* feat: Group API in core and extensions (#23)

Signed-off-by: Thomas Poignant <[email protected]>

* Update guideline/static-context-provider.md

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>

* Update openapi.yaml

Signed-off-by: Thomas Poignant <[email protected]>

---------

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Michael Beemer <[email protected]>
Signed-off-by: Roman Dmytrenko <[email protected]>
Signed-off-by: Kavindu Dodanduwa <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Honza Dvorsky <[email protected]>
Co-authored-by: Mark Phelps <[email protected]>
Co-authored-by: Michael Beemer <[email protected]>
Co-authored-by: Kavindu Dodanduwa <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Lukas Reining <[email protected]>
Co-authored-by: Michel TURPIN <[email protected]>
Co-authored-by: Roman Dmytrenko <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
Co-authored-by: Honza Dvorsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@lukas-reining lukas-reining lukas-reining approved these changes

@thomaspoignant thomaspoignant Awaiting requested review from thomaspoignant

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Kavindu-Dodan @beeme1mr @lukas-reining