Skip to content

Share a key found in the request and the Evidence once, whatever its letters - #120

Merged
jwrosewell merged 2 commits into
rcfrom
fix/share-usage-key-case
Nov 14, 2026
Merged

jwrosewell merged 2 commits into
rcfrom
fix/share-usage-key-case

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ShareUsage.found_evidence merges the values of the web request, which the web integration adds as server.http-request, with every Evidence value in the Flow Data, so that a key found in both places is shared once with the Evidence value (share_usage.py lines 432 to 458 at 32ffdcf1). It merged on the exact key. webevidence writes each header's key in lower case (web.py line 91), so a header an application added by hand in other letters, such as header.User-Agent, was found twice and both values were shared.

What the specification says

usage-sharing-element.md, line 29, "A key found in both places is shared once, with the value from the Evidence.", and lines 35 and 36, "Keys are compared without regard to case." evidence.md line 10 says "Keys MUST be case-insensitive".

What changed

  • found_evidence keys what it finds by the lower case form of each key, so a key found in more than one place is found once, with the value and the spelling of the last place, the Evidence's. Its documentation says so.
  • Two tests in test_share_usage_accepts_no_evidence.py read the message the element actually sends, by closing the element, which sends what it has queued, with requests.post replaced by one that keeps the body and answers 200. test_a_key_in_both_places_whose_letters_differ_is_shared_once adds HEADER.User-Agent by hand and header.user-agent in the request, as 51degrees-node's shareUsageAcceptsNoEvidence.test.js does at line 279, and expects one user-agent header in any letters, with the Evidence value. test_a_header_no_element_reads_is_in_the_message_sent builds a real Flask request with an X-Test header that no element reads, passes it through webevidence into a Pipeline with a User-Agent element and the usage sharing element, and finds <header Name="x-test"> in the message sent, which the earlier tests checked only in the queue.

How it was verified

Both runs are temporary branches holding a workflow that installs the pipeline packages from the checkout into a virtual environment of the runner's own Python (3.12.3 on Ubuntu, 3.12.10 on Windows) and runs the listed tests with pytest, on ubuntu-latest and windows-latest.

  • Red, rc at 32ffdcf with the two new tests and no change to found_evidence, commit f99341ab, run 43521235919, failed on both with 1 failed, 8 passed, the failure being test_a_key_in_both_places_whose_letters_differ_is_shared_once with 1 != 2, two user-agent headers in the message sent. test_a_header_no_element_reads_is_in_the_message_sent passed on rc as well, because the header already reached the message, and it is there to show that it reaches the message sent and not only the queue.
  • Green, this head ddec4a5e, run 43521248093, passed on both, 9 passed, then the whole fiftyone_pipeline_engines_fiftyone suite (86 passed) and the whole fiftyone_pipeline_core suite (481 passed).
  • Full build, run 43521977174, started by tools/pr-matrix.py on tmp/pr-matrix-120, finished with success. Configure printed "Left out 5 macOS entries", and the 11 Setting 'PullRequestSha' lines (Configure and the ten Build and Test jobs, Python 3.10 to 3.14 on Ubuntu and Windows) all give ddec4a5. macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
  • Quick checks passed on this head, being run 43521956608 (environment variable names), run 43521956923 (only rc into main) and run 43521956387 (UTM link lint).

share_usage.py is in fiftyone_pipeline_engines_fiftyone's source, so the release plan releases that package and the packages that depend on it.

The last release into main was #114, merged at 06:22 UTC on 13 November.

@jwrosewell
jwrosewell marked this pull request as ready for review November 14, 2026 00:12
@jwrosewell
jwrosewell merged commit 63566d0 into rc Nov 14, 2026
3 checks passed
@jwrosewell
jwrosewell deleted the fix/share-usage-key-case branch November 14, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant