fix: treat ttl_seconds as an ignored push_config field - #587
Conversation
ttl_seconds is a field of the server-side push API (PushClientCommandRequest), not of ClientCommand, so it never reaches clients by design. Stop validating its type and report it as ignored like any other unknown field, so a stray value can no longer fail the whole command. Co-Authored-By: opencode <noreply@opencode.ai> Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xiaofan-luan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Tick the box to add this pull request to the merge queue (same as
|
| {{"push", "push_config", R"({"enabled":"false"})", 1, false, ""}, | ||
| {"collection", "collection_metrics", R"({"enabled":false,"collections":"books"})", 2, false, ""}, | ||
| {"ttl", "push_config", R"({"enabled":true,"ttl_seconds":"bad"})", 3, false, ""}}); | ||
| {"ttl", "push_config", R"({"ttl_seconds":"bad"})", 3, true, ""}}); |
There was a problem hiding this comment.
test/ut/TestClientTelemetry.cpp line:1022
Low ---- The persistent flag on this command changed from false to true (with enabled removed from the payload) without a comment. Since ProcessCommands computes CalculateConfigHash for persistent commands, this hash now incorporates the {"ttl_seconds":"bad"} payload. The test does not assert the config hash, so this is harmless here, but could you confirm whether the flag change is intentional (e.g. to mirror a real persistent push_config) and, if so, worth a brief note in the test?
3 participants
Summary
ttl_secondsis a field of the server-side push API (PushClientCommandRequest/POST /_telemetry/commands), not ofClientCommand, so it never reaches clients by design (see the TTL section of the design doc).push_configcommand handler and report it as ignored like any other unknown field, so a stray value can no longer fail the whole command.RejectsWrongCommandPayloadTypesaccordingly: an invalidttl_secondsnow succeeds withignored: ["ttl_seconds"]and leaves the applied config untouched.This is a follow-up to #585 (the change was pushed to the head branch shortly after merge and did not land).
pr: #585
Related work
Verification
ClientTelemetryTest.*26/26 passed