Skip to content

Conversation

iamafanasyev
Copy link
Contributor

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

#613

How Has This Been Tested?

  1. make test
  2. make all:

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2025

⚠️ Please install the 'codecov app svg image' 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 ⚠️

❗ 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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

github-actions bot commented Jul 7, 2025

Stale pull request message

@iamafanasyev
Copy link
Contributor Author

@nothing0012 hey there

@iamafanasyev
Copy link
Contributor Author

or @zhouzhuojie mb :)

Copy link

Stale pull request message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants