flagr
flagr copied to clipboard
Extend evaluation API with GET requests
Description
Introduce both GET /evaluation and GET /evaluation/batch (functionally equivalent to POST /evaluation and POST /evaluation/batch).
Due to the URL length limits request query parameters were "length optimised" (instead of keeping them the same as in POST body: enableDebug -> dbg, flagIDs -> ids, flagTagsOperator ~> all, etc.). In fact, this is the main subject of the review.
Evaluation entities are encoded via JSON string representation (property names are also "length optimised"):
?entity={"id":"id1","type":"t1","ctx":{"hello":"world"}};?entity=%7B%22id%22%3A%22id1%22%2C%22ctx%22%3A%7B%22hello%22%3A%22world%22%7D%7D;?entity={"id":"id1","ctx":{"foo":"bar"}}&entity={"id":"id2","ctx":{"baz":"qux"}}.
This evaluation entity serialisation is used in both /evaluation and /evaluation/batch routes as:
- it provides more clear semantics for flag filter "length optimised" parameter names (
/evaluation?entity={"id":"id1","ctx":{"foo":"bar"}}&id=1&key=flag-1); - it provides universal entity definition for both routes (nice to have, as it is already quite complicated).
All array query parameters (up to entity) use collectionFormat: csv also for "length optimisation" purpose. entity uses collectionFormat: multi, as collectionFormat: csv does not work properly for the selected encoding
Motivation and Context
https://github.com/openflagr/flagr/issues/613
How Has This Been Tested?
make testmake all:
- POST requests from
Debug consolesection from Flagr UI ("regress" preexisting evaluation routes); - without URL encoding: http://127.0.0.1:18000/api/v1/evaluation?entity={%22id%22:%22foobar%22,%22type%22:%22a%22,%22ctx%22:{%22foo%22:3.5,%22baz%22:[1,2]}}&id=1&tags=foo,bar&all=1&dbg=1;
- with URL encoding: http://127.0.0.1:18000/api/v1/evaluation/batch?entity=%7B%22id%22%3A%221245%22%2C%22type%22%3A%22user%22%2C%22ctx%22%3A%7B%22foo%22%3A3.5%2C%22baz%22%3A%5B2%5D%7D%7D&dbg=1&tags=foo,bar
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
Attention: Patch coverage is 86.31579% with 13 lines in your changes missing coverage. Please review.
Project coverage is 76.63%. Comparing base (
770461b) to head (c5ac784). Report is 9 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pkg/handler/eval.go | 86.02% | 9 Missing and 4 partials :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## main #616 +/- ##
==========================================
- Coverage 81.19% 76.63% -4.56%
==========================================
Files 28 30 +2
Lines 2271 2487 +216
==========================================
+ Hits 1844 1906 +62
- Misses 337 487 +150
- Partials 90 94 +4
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Stale pull request message
@nothing0012 hey there
or @zhouzhuojie mb :)
Stale pull request message
ping