Repository navigation
Set the client-side cache headers, with an ETag that is the same in every process and none after errors - #59
Draft
jwrosewell wants to merge 6 commits into
Draft
jwrosewell wants to merge 6 commits into
jwrosewell wants to merge 6 commits into
Conversation
…very process and none after errors
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.
For specifications#53, the client-side caching change decided on 5 November.
The problem
The cache header detail of the web integration feature says the JSON and JavaScript endpoints MUST set
Cache-Control: max-age=1800 private,Vary, anETagthat hashes the Evidence,Content-TypeandContent-Length, and answer a matchingIf-None-Matchwith a 304. specifications#53 adds that the same Evidence MUST give the same ETag in every process running the same build, and that a response to a request whose processing recorded errors MUST carryCache-Control: private, no-cacheand no ETag, while a request whoseIf-None-Matchmatches still gets its 304.Python has no web integration package. The client-side endpoints are written in each Flask example, and none of them set any cache header. The places that build them are:
/jsonand/51Degrees.core.jsindevice-detection/.../cloud/gettingstarted_web/app.pyand in the on-premise copy beside it,/51Degrees.core.jsand/jsonin the pipeline's mixed cloud example,.../cloud/mixed/gettingstarted_web/app.py,/jsoninlocation/examples/cloud/web.pyand inpipeline/fiftyone_pipeline_core/examples/client_side_evidence_custom_flow_element.py.The one shared module is
fiftyone_pipeline_core/web.py, which the examples already use forwebevidenceandset_response_header.The change
fiftyone_pipeline_core.webgainsjavascript_response(pipeline, request)andjson_response(pipeline, request), which process the request and answer with the JavaScriptBuilder's JavaScript or the JSONBundler's JSON. Everything else is private to the module.hash()of a string is seeded afresh in each process, so it is not used.If-None-Matchis compared with the weak comparison RFC 9110 section 13.1.2 asks for, soW/is ignored, a list matches when any member does, and*matches. A match on a GET or HEAD is answered 304 without processing, with theETag,Cache-ControlandVarya 200 would have carried, as section 15.4.5 requires, and noContent-Type. Python's endpoints set no CORS headers on a 200, so the 304 has none either. A match on any other method, being the POST to the JSON endpoint, is answered 412 Precondition Failed with no body and no ETag, as section 13.1.2 requires. James decided on 7 November to follow RFC 9110 here in every language, rather than the specification's older text, which gave both endpoints a 304.Content-Type(application/x-javascriptorapplication/json),Content-Lengthfrom Flask, andCache-Control: max-age=1800, privatewith the ETag, orCache-Control: private, no-cacheand no ETag whenflowdata.errorsis not empty.Varynames the request's headers that one of the pipeline's elements accepts.If-None-Match,If-Modified-Since,If-Match,If-Unmodified-Since,If-Range,Cache-Control,Pragma) are left out of the ETag and ofVary. An element whose filter accepts every key would otherwise putIf-None-Matchin the ETag, and a revalidation would never match the response it revalidates. Accept only the evidence each element reads, and none by default #63 narrows the four default elements that did, so a default on-premise pipeline no longer accepts these headers. Accept only the evidence each element reads, and none by default #63 also has the cloud engines accept nothing. The list stays, becauseFlowElement's default filter still accepts every key, so a pipeline holding an element that does not set its own filter, such as a customer's, would still break revalidation without it.The five example routes now call these two functions. Their comments say what the call does.
How to verify
Red is
rcat 0507eef with only the tests added (d4e82a1), and green is this branch at b1a6754. Both ran the same temporary workflow on Ubuntu and Windows with Python 3.12, runningtests/test_web_client_side.py,tests/test_web.pyandtests/test_javascriptbuilder.pyinpipeline/fiftyone_pipeline_coreandtests/test_cloud_mixed_web_offline.pyin the pipeline examples, which runs the mixed example against a stand-in for the cloud service.Red (run 42394701136, failure on both). The examples' endpoints send only two headers today.
The core tests could not be collected, with
ImportError: cannot import name 'javascript_response' from 'fiftyone_pipeline_core.web'.Green (run 42395023618, success on both).
Windows gave the same counts.
The 412 for a POST. Red is this branch at b1a6754 with only the tests changed (4c36e9a), and green is 8e8f285, on the same workflow (red run 42416498061, failure on Ubuntu and Windows, green run 42416617410, success on both).
Green gave 25 passed in the core tests and 18 in the examples on both. The core test of the 304 now sends the revalidation as a GET and as a HEAD and checks that its
ETag,Cache-ControlandVaryare those of the good response. The full build in step 3 ran before this change.The web request value. The usage sharing change (Accept no evidence for usage sharing, and share what it finds in the flow data and the web request #61) has
webevidence()add the request itself asserver.http-request, a read-only view of every header,If-None-Matchincluded. The JavaScript builder accepts every key (Three elements the pipeline builder adds accept every evidence key #52), so hashing it would give a revalidation a different ETag._uses()now leaves it out. Red is 300a081's parent dcde28f, green 300a081 (red run 42418120887,AssertionError: 304 != 200on Ubuntu and Windows, green run 42418163568, 26 passed in the core tests and 18 in the examples on both).The tests check each point of the specification for both endpoints. The cross-process tests run the endpoint in two new Python processes with
PYTHONHASHSEEDset to 1 and to 2 and compare the ETags. The failure tests use an element, or a stand-in cloud service, that fails while a flag is set, so the same evidence is processed first with and then without errors.nightly-pull-request.ymlrun as a dry run on this pull request over all 15 entries inci/options.json(run 42400101270, success, 17 jobs passed and the two performance comparison jobs skipped). On Ubuntu with Python 3.12 (job) the pipeline core unit tests gave 294 passed, the Device Detection examples 31 passed, the pipeline examples 38 passed and 13 skipped (the licence key half), location 3 passed, and the Selenium contract tests passed 9 of 9 against the Device Detection cloud web example and 9 of 9 against the on-premise one, both now serving/51Degrees.core.jsand/jsonthrough the new functions.For James's review
Varynamed every header the request carried. With Accept only the evidence each element reads, and none by default #63 a default pipeline names only the headers its engines read andHost(Vary: Host, User-Agentin Accept only the evidence each element reads, and none by default #63's check with this merged in). A pipeline holding an element that does not set its own filter still names every header, becauseFlowElement's default filter accepts every key. That is safe, because the browser only reuses its copy when they all match.What remains
fiftyone_pipeline_coregains two public functions, so it is released with this change and so are the packages that depend on it.