Make acquire a library a second project can consume, and check the walk it hands back - #75
Merged
Merged
Conversation
…lk it hands back wildfire-service-territory-overlap pins a commit of this package and audited the seam on 2026-09-05, recording four gaps in its docs/UPSTREAM.md. All four are closed here. A caller can say who it is. _get, layer_record_count, fetch_layer and acquire take a user_agent, defaulting to this project's. The walk used to send perimeter-coverage whoever was calling, so an operator reading CAL FIRE's logs saw the library rather than the caller. A blank User-Agent is refused rather than passed through: urllib would substitute Python-urllib/3.x, which identifies nobody. iter_features exposes the paged walk with return_geometry and out_sr, because the consumer needed geometry and copied the offset loop to get it, including the subtle part that steps by the page that arrived rather than the page that was asked for. A feature is yielded whole rather than merged into its attributes: a layer may publish a field called geometry and a merge would overwrite it. The default request is unchanged byte for byte, so the raw files pinned in sources.py stay reproducible, and outSR is absent rather than defaulted. The layer's total is read twice, before the walk and after it. One count read before a walk cannot see a layer republished while the walk was running; the walk ends at a total that matches an hour ago and the file is a mixture of two versions with a clean hash on it. Both numbers are named in the refusal. The identifiers are checked too, because a count that matches is not evidence that the right records were collected. A page handed back twice, or a result reordered mid-walk, leaves the count intact and the contents wrong. identifier_failure refuses a repeat, a value that goes backwards, and a non-integer, True included since it is an int in Python. acquisition.json records which guards ran, so a manifest written before the post-walk recount existed is distinguishable from one written after it. src/perimeter/py.typed, checked by running mypy --strict over a minimal consumer rather than by asserting the file exists: shipped and honoured is the fact that deletes the consumer's override.
This was referenced Sep 7, 2026
Closed
ChelseaKR
added a commit
that referenced
this pull request
Sep 8, 2026
…ot read is refused (#85) ## What was wrong `iter_features` gained `return_geometry` and `out_sr` in #75, which closed most of the consuming project's acquisition gap. It kept `"f": "json"` hard-coded, and that was the argument the duplication stayed alive: `wildfire-service-territory-overlap` reads four layers and three of them as `f=geojson`, which this walk could not be asked for. So its own `fetch_feature_pages` stays, and with it a second copy of the offset rule this package exists to stop being copied, and a second copy of this module's refusals about somebody else's server. Two smaller things were found while measuring it: - The walk is **not** format-agnostic, though it reads as though it is. It reads `features` and `exceededTransferLimit` out of the top level of every answer. A JSON-shaped format carrying neither would yield nothing on its first page and stop — with no exception, a clean hash, and a record count of zero. That is indistinguishable from a layer holding no records. - `payload.get("features", [])` made the same two facts one value. "The service answered something this walk cannot read" and "the layer holds no records" both produced an empty list, and the walk ended quietly on either. The record count copied out of the resulting file is published on both pages and pinned in `sources.py`. ## What changed - `iter_features(..., out_format: str = DEFAULT_OUT_FORMAT)`, threaded into the query dict beside `out_sr`. A caller asking for `geojson` receives the service's own `Feature` objects, whole: nothing merged, nothing renamed, no geometry converted. - `PAGEABLE_OUT_FORMATS` and `UnpageableFormatError`. A format the walk cannot page is refused **before a socket opens**, with a message naming what the walk reads and why an unpageable format would look like an empty layer. - A page with no `features` key raises `AcquisitionFailed` naming the offset and the keys that were there. An empty `features` list still ends the walk cleanly, and that side of the boundary has its own test so the refusal cannot widen into one that refuses honest emptiness. - `fetch_layer` deliberately takes no `out_format`, and its signature is pinned by a test saying why: it reads `feature["attributes"]`, which a GeoJSON `Feature` does not carry. The default request has not moved. `test_the_default_request_is_the_one_this_project_has_always_made` now names `f=json` alongside `returnGeometry=false` and the absent `outSR`. ## How it was verified `make verify` green in full, determinism gate included. Three negative controls, each committed-baseline / sabotage / `git hash-object` re-assert, `__pycache__` cleared between runs, and each fired on exactly the tests predicted and no others: | sabotage | predicted | measured | |---|---|---| | restore `payload.get("features", [])` — the state `origin/main` is in | the missing-key refusal only | **1 red**, that test | | `"f": out_format` back to `"f": "json"` — the parameter accepted and ignored | both geojson tests, default-request test green | **2 red**, exactly those | | the format refusal never runs | the unpageable-format test only | **1 red**, that test | The second is the one worth naming: a parameter that is accepted and silently dropped looks exactly like one that works, and only the two tests that read `f` off the sent URL can tell them apart. The paging rule is exercised under `f=geojson` rather than assumed to be shared — a capped layer of 5,000 records served 1,000 at a time, walked to completion. The reason to expose the walk at all is that the offset rule stops being copied; a non-default format reaching the same records by an untested path would leave the consumer with a second implementation of the subtle part after all. The planted unpageable format is `f-that-no-geoservices-layer-will-ever-publish`, not the next plausible format name, and the test asserts it is absent from `PAGEABLE_OUT_FORMATS` — a planted value drawn from the namespace the code grows into has a shelf life nobody writes down. (Body truncated; full rationale is in the pull request.) Prepared with AI assistance; reviewed before submission.
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.
Delivers every item listed in issue #64. Leaving that issue open for the owner to check off; nothing here needs her before it lands.
What was wrong
wildfire-service-territory-overlappins a commit of this package and audited the seam on 2026-09-05, recording four gaps in itsdocs/UPSTREAM.md. All four are confirmed present onmainand closed here, so the consumer's compensations can be deleted at its next pin.mainbefore thisacquire.py:118sendsUSER_AGENTunconditionally; neitherlayer_record_countnorfetch_layertakes onefetch_layer(endpoint, fields)— no geometry, no format, no spatial reference, no page iteratorpy.typed, so a strict consumer carries two mypy overridesWhat changed
A caller can say who it is.
user_agenton_get,layer_record_count,fetch_layerandacquire, defaulting to this project's. An operator reading CAL FIRE's logs sawperimeter-coveragefor requests a different project made — the header identified the library rather than the caller, which is the one thing a User-Agent exists to do.A blank User-Agent is refused rather than passed through. urllib would substitute its own
Python-urllib/3.x, so the request still goes out under a header naming no project at all: an absent identity sent as though it were one.iter_featuresexposes the paged walk, withreturn_geometryandout_sr. The whole value of that loop is one rule — step by the page that arrived, not the page that was asked for, because a layer capping a page belowPAGE_SIZEand settingexceededTransferLimitleaves a block of records between the end of this page and the next offset, and stepping byPAGE_SIZEwalks over them while the walk still ends normally. A copy of that rule in another repository drifts.A feature is yielded whole, not merged into its attributes: a layer is free to publish a field called
geometry, and a merge would overwrite it silently — a loss that looks exactly like a layer that never published the field. There is a test for that case.The default request is unchanged byte for byte (
returnGeometry=false, andoutSRabsent rather than defaulted), because the raw files pinned insources.pymust stay reproducible.The layer's total is read twice, before the walk and after it. One count read before a walk cannot see a layer republished while the walk was running: the walk ends at a total matching the number the layer held an hour ago, the pre-walk check passes, and a file assembled across two versions of a layer lands on disk with a clean hash and a date beside it. Two counts that disagree say exactly that, and the refusal names both.
The identifiers are checked, because a count that matches is not evidence that the right records were collected. The walk asks for
OBJECTID ASCand steps an offset through the answer. A page handed back twice, or a result reordered under a concurrent edit, leaves the count intact and the contents wrong — some records collected twice and others not at all.identifier_failurerefuses a repeat, a value that goes backwards, and a non-integer.Trueis in the test table on purpose: it is anintin Python and would slip past anisinstance(value, int)check written without the guard.Ordering on a field and then never checking the order is a check that cannot fail, which is what
mainhad.acquisition.jsonrecords which guards this acquisition passed, not which ones the code contains. A manifest written before the post-walk recount existed and one written after it were otherwise indistinguishable, and a file that passed the recount is a different claim from a file that did not.src/perimeter/py.typed, plusartifactsin the wheel target so it ships. This makes the annotations a published interface for the project that pins this one: a later signature change becomes a breaking change for them, which is the intent and is stated in the CHANGELOG.Done-when, checked
_getsends, and the default still names this project3 records before the walk/4 after itmypy --strictover a minimal consumer importingperimeter.acquirepasses with no overridesHow it was verified
843 passed, coverage 100.00% total with branch coverage on (floor 90),acquire.pyat 100% statements and 100% branches.mypy --strictclean over 12 source files.ruff check/ruff format --checkclean.uv lock --checkclean.Five negative controls, each with the sabotage asserted present in the file before the result was read, and each restored from a byte copy whose SHA-256 was compared afterwards:
src/perimeter/py.typeddeletedlsreported it absentmissing library stubs or py.typed markerafter = before)grep -nshowed the replacement line…republished_mid_walk…FAILED,DID NOT RAISEidentifier_failurereturnsNoneimmediatelygrep -nshowed the earlyreturn None_getsendsUSER_AGENTinstead of the caller'sgrep -nshowedUSER_AGENTin the header…say_who_it_is…FAILED, positive control still passedreturnGeometryhard-coded to"false"grep -nshowed the literalRestored,
55 passedintests/test_acquire.py.Note on the existing tests
The fakes in
tests/test_acquire.pytook_get(url)positionally, so they now accept**_. That is the only change to existing test bodies; no assertion was altered or removed. The autouse fixture that refuses a real socket is untouched and still enforces that nothing here contacts CAL FIRE.Out of scope, as the issue says
Changing what this project acquires by default — nothing in
sources.py, no re-acquisition, no published figure moves. And publishing to PyPI under a new name, which is a separate decision.Prepared with AI assistance; reviewed before submission.