Repository navigation
Share a key found in the request and the Evidence once, whatever its letters - #120
Merged
Merged
Conversation
…d for a key in both places in other letters
…uest and the Evidence
jwrosewell
marked this pull request as ready for review
November 14, 2026 00:12
This was referenced Nov 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ShareUsage.found_evidencemerges the values of the web request, which the web integration adds asserver.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.pylines 432 to 458 at 32ffdcf1). It merged on the exact key.webevidencewrites each header's key in lower case (web.pyline 91), so a header an application added by hand in other letters, such asheader.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_evidencekeys 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.test_share_usage_accepts_no_evidence.pyread the message the element actually sends, by closing the element, which sends what it has queued, withrequests.postreplaced by one that keeps the body and answers 200.test_a_key_in_both_places_whose_letters_differ_is_shared_onceaddsHEADER.User-Agentby hand andheader.user-agentin the request, as 51degrees-node'sshareUsageAcceptsNoEvidence.test.jsdoes at line 279, and expects oneuser-agentheader in any letters, with the Evidence value.test_a_header_no_element_reads_is_in_the_message_sentbuilds a real Flask request with anX-Testheader that no element reads, passes it throughwebevidenceinto 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-latestandwindows-latest.rcat 32ffdcf with the two new tests and no change tofound_evidence, commit f99341ab, run 43521235919, failed on both with1 failed, 8 passed, the failure beingtest_a_key_in_both_places_whose_letters_differ_is_shared_oncewith1 != 2, twouser-agentheaders in the message sent.test_a_header_no_element_reads_is_in_the_message_sentpassed onrcas 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.9 passed, then the wholefiftyone_pipeline_engines_fiftyonesuite (86 passed) and the wholefiftyone_pipeline_coresuite (481 passed).tools/pr-matrix.pyontmp/pr-matrix-120, finished with success. Configure printed "Left out 5 macOS entries", and the 11Setting '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.share_usage.pyis infiftyone_pipeline_engines_fiftyone's source, so the release plan releases that package and the packages that depend on it.The last release into
mainwas #114, merged at 06:22 UTC on 13 November.