Repository navigation
Accept no evidence for usage sharing, and share what it finds in the flow data and the web request - #61
Draft
jwrosewell wants to merge 3 commits into
Draft
Accept no evidence for usage sharing, and share what it finds in the flow data and the web request#61jwrosewell wants to merge 3 commits into
jwrosewell wants to merge 3 commits into
Conversation
This was referenced Nov 7, 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.
Follows specifications#99, the draft that has the usage sharing element accept no Evidence, as James decided on 7 November for Monday's release.
The problem
ShareUsageEvidenceKeyFilterwas both what the usage sharing element accepted and what it shared, so it accepted every header exceptCookie, everyserver.key and the other keys its rules share (share_usage_evidencekeyfilter.py,share_usage.pyline 129). A pipeline accepts a key when any element does, so a pipeline holding usage sharing acceptedIf-None-Matchand every other header. The client-side ETag (#59) and the Vary header are built from what the pipeline accepts, so they named every header and the ETag of a revalidation could never match. The engines' own caches here use each engine's own filter, so they did not depend on it.The change
ShareUsage.get_evidence_key_filter()returns an emptyBasicListEvidenceKeyFilter.ShareUsageEvidenceKeyFilterandShareUsageTrackerEvidenceKeyFilterare unchanged, and are now only the rules for which found keys are shared and which tell a repeat apart.ShareUsage.found_evidence()returns the values of the web request underserver.http-request, then every evidence value in the flow data, whether an element accepted it or not, with a later source replacing an earlier one, so a key found in both places is shared once with the evidence value.server.http-requestitself is never among them. Both the data sent and the tracker's hash are now built from these found values, so the hash is of the values that would be shared.Evidence.filter(), whichFlowData.process()runs before any element, used to delete each key no element accepts. With usage sharing accepting none, that would delete every header only usage sharing read, before it could read it, including evidence a caller adds by hand, which the brief says must still be shared. Sofilter()now moves those keys to a newEvidence.get_unaccepted(), andget_all()holds what it held before for every other element.server.http-request.fiftyone_pipeline_core.web.webevidence()adds a read-only view, atypes.MappingProxyType, of the headers, cookies, query string and form fields it read, keyed as evidence is (header.<name>,cookie.<name>,query.<name>). It does not add the Flask request object itself, because the other elements readget_all()as a dictionary of strings. The Device Detection and IP Intelligence on-premise engines pass every value to their native evidence map, and the cloud request engine sends every key that is not a header, cookie or query value, so a request object there would break the first and be sent by the second. A view keyed as evidence is also what the element already knows how to read. Session values are not in the view, becausewebevidence()has never read the Flask session and usage sharing has never shared one, and the brief says nothing else about what is shared changes.server.http-requestreaches no other element.Evidence.filter()always sets it aside, whatever the filters accept, because three elements the builder adds accept every key until Three elements the pipeline builder adds accept every evidence key #52 is fixed.HTTP_REQUEST_KEYinfiftyone_pipeline_core.evidencenames it.share_usage.pywrites the key out rather than importing it, and readsget_unaccepted()only when it exists, so a newerfiftyone_pipeline_engines_fiftyonestill loads beside an olderfiftyone_pipeline_core, sharing less.What is shared is the same as before for the same request. A header only usage sharing read moves from
get_all()toget_unaccepted(), and the element reads both.The other elements that accept every key
#52 found that the sequence element, the JavaScript builder and the set header element accept every key, and the JSON bundler does too. With usage sharing accepting none, they were the only reason a default pipeline still accepted every header. James decided on 7 November that they narrow, which #63 does as a draft of its own, and its check with #59 merged in gives
Vary: Host, User-Agentfor a default pipeline.CloudEngineand the cloud aspect engines built on it still accept every key, which #63 asks about.How to verify
Red is
rcat 0507eef with only the tests added (0f6be78), and green is this branch. Both ran the same temporary workflow on Ubuntu and Windows with Python 3.12, running every test offiftyone_pipeline_core,fiftyone_pipeline_engines_fiftyoneandfiftyone_pipeline_cloudrequestengine, andtests/test_cloud_mixed_web_offline.pyof the pipeline examples.If-None-Matchfor usage sharing, so a revalidation's accepted evidence differed.rcas well, because they check that what is shared does not change.fiftyone_pipeline_core287 passed,fiftyone_pipeline_engines_fiftyone40 passed and the pipeline examples' offline tests 12 passed, on both. The run reads as a failure only because 4 tests in the cloud request engine'stest_cloudengine.pyfailed with "You need to create a resource key", onrcin the red run as on this branch. They call the live cloud service and the workflow gave them no key. The full pull request build, which gives them one, is below.A first green run on 89347b9 (run 42418000569) failed
test_session_cookieintest_usage.py, which asserted that the element accepted the session cookie. 352c3f9 changes that assertion, as described below.nightly-pull-request.ymlrun as a dry run on this pull request at 352c3f9 over all 15 entries inci/options.json(run 42418841312, success, 17 jobs passed and the two performance comparison jobs skipped). On Ubuntu with Python 3.12 (job) the cloud request engine's tests, given the Bespoke key, gave 40 passed and 2 skipped (the licence key tests), the Device Detection cloud tests 14 passed, the Device Detection examples 31 passed, the IP Intelligence examples 20 passed and 3 skipped, and the Selenium contract tests passed 9 of 9 against each Device Detection web example, the on-premise one holding usage sharing. The on-premise engines, which pass every evidence value to native code, ran withwebevidence()adding the web request.The brief's four tests are these.
test_the_pipeline_accepts_no_header_for_usage_sharingbuilds a pipeline of the usage sharing element and an element acceptingheader.user-agentalone, and checks that neitherheader.if-none-matchnorheader.x-testis accepted.test_the_web_request_is_shared_by_the_same_rulessharesX-Test, which no other element accepts, a51D_cookie and a51D_query parameter, and leaves out theCookieheader, a blocked header, an ordinary cookie, a query parameter not on the list andserver.http-request.rc, and Set the client-side cache headers, with an ETag that is the same in every process and none after errors #59 adds one, so the test here,test_a_revalidation_has_the_same_accepted_evidence, checks the evidence the ETag hashes, which is what the pipeline accepts. A request and its revalidation carryingIf-None-Matchgive the same accepted evidence. With Set the client-side cache headers, with an ETag that is the same in every process and none after errors #59,test_revalidation_with_the_etag_is_answered_not_modifiedthere checks the 304 itself, andtest_the_web_request_value_is_not_in_the_etagthere checks thatserver.http-requestdoes not change the ETag.test_evidence_added_by_hand_is_shared_as_beforeadds a header no element accepts, a51D_cookie, an ordinary cookie and the client IP by hand, with no web request, and they are shared as before. The existing tests intest_usage.py, which add evidence by hand to a default pipeline, all pass.test_a_key_in_both_places_is_shared_once_with_the_evidence_valueandtest_a_repeat_is_told_apart_by_what_would_be_sharedcheck the rest of the brief.test_evidence_unaccepted.pychecks thatfilter()sets aside what no element accepts, and thatserver.http-requestis added and set aside even when an element accepts every key.test_the_web_request_is_not_sent_to_the_cloudin the pipeline examples checks that the cloud request engine never sends it.test_session_cookieintest_usage.pyasserted that the element accepted the session cookie. It now asserts that the element does not, and that the tracker's filter reads it.Other drafts on the same files
share_usage.pyandtest_usage.py.fiftyone_pipeline_core/web.pyand adds tests at the end oftest_cloud_mixed_web_offline.py, as this does. Set the client-side cache headers, with an ETag that is the same in every process and none after errors #59 leavesserver.http-requestout of its ETag, so the two work in either order.What remains
fiftyone_pipeline_coreandfiftyone_pipeline_engines_fiftyonechange, so they are released, and so are the packages that depend on them.