Skip to content

[pipeline-core #339] Bump the pin for a Tier-1 stale-run fix; declare your own appwrite dependency #172

Closed

Description

@Polichinel

Filed from views-pipeline-core's epic views-platform/views-pipeline-core#339. Two small asks; the first is free.

1. Bump the pipeline-core pin — you inherit a Tier 1 fix with no code change

pipeline-core's search_files_by_metadata was unpaged: Appwrite returns 25 rows by default, and get_latest_file_id sorted those and called the result "latest" — so with more than 25 matching documents it returned the newest of the oldest 25, silently, and staler every run.

This reaches you. _ContractStorePort.latest_file_id (views_postprocessing/unfao/managers/unfao.py) feeds ADR-013 resolve_run, whose manifest lookup uses the broad HOP_A_MANIFEST_FILTERS = {"category": "forecast", "type": "sampled_forecast_manifest"} — every manifest ever published matches. Shard lookups are name-scoped and unique, so they resolve fine for whichever run was selected. Net effect: the delivery can assemble and ship a stale run rather than failing.

Because you import pipeline-core's DatastoreModule rather than keeping your own client, the fix arrives with a pin bump — no code change on your side. Tracked as views-pipeline-core C-241; fixed in #341.

Ask: bump the pin once #341 merges, and confirm a delivery selects the run you expect.

2. Declare your own appwrite dependency

You use Appwrite (5 code references) but declare it nowhere — you free-ride on pipeline-core's transitive dependency. pipeline-core is making appwrite an optional extra (#345), because three repos that never mention Appwrite currently install its SDK (CRP). Once that lands, the transitive path disappears.

Ask: declare appwrite in your own pyproject.toml, or depend on views-pipeline-core[appwrite].

Not an ask — a compliment worth recording

Your _ContractStorePort docstring — "DIP — wire/source_selection and wire/sink never see Appwrite types" — describes exactly the architecture this epic is trying to reach elsewhere. Of your ~826 Appwrite-adjacent lines, only ~130 are genuinely vendor-coupled; the rest is FAO delivery logic that belongs with you. Nothing structural is being asked of you.

Activity

  1. Polichinel commented on Aug 1, 2026

    @Polichinel
    CollaboratorAuthor

    pipeline-core #341 has merged — and it carries a breaking change you should know about before bumping.

    The Tier-1 fix is in: search_files_by_metadata now pages, so get_latest_file_id can no longer return the newest of the oldest 25 matches. You inherit that with the pin bump and need no code change for it.

    What is new since this issue was filed: fixing the paging exposed a second half. DatastoreModule.get_predictions_by_metadata and list_all_predictions_unfiltered used to return [] when the search failed, which get_latest_file_id turned into None. Left alone, the paging fix would have traded a false-stale answer for a false-absent one. Both now raise MetadataSearchIncomplete (a RuntimeError subclass, so existing broad handlers still catch it).

    Concretely for this repo: contract/wire/source_selection.py:106 already raises SourceSelectionError when store.latest_file_id(...) returns None, with the message

    no Hop-A manifest in the store (filters=...) — nothing has been published under the contract.

    That message is true for a real absence and false for a broken read, and until now you had no way to tell the two apart. After the bump you will get MetadataSearchIncomplete propagating through _ContractStorePort.latest_file_id (unfao/managers/unfao.py:33) instead. Both outcomes are loud failures, so there is no silent regression — but the diagnosis is now honest, and you may want to catch it explicitly at the port boundary and re-raise as a distinct SourceSelectionError variant so the delivery log says "could not read the store" rather than "nothing has been published".

    One caveat worth knowing: the walk is certified against the server's reported total, so a document written during the walk also yields MetadataSearchIncomplete. That is deliberate — a count nobody verified must not be delivered — but it means the exception is retryable, and nothing retries it today.

    Nothing here blocks the bump. Filed so the behaviour change is not a surprise.

  2. Polichinel commented on Aug 1, 2026

    @Polichinel
    CollaboratorAuthor

    Upgrading this from hygiene to a hard break — please read before your next clean install.

    pipeline-core #361 merged: appwrite is now an optional extra, not a hard dependency. Measured consequence for this repo specifically:

    views_postprocessing/unfao/managers/unfao.py:7  from views_pipeline_core.modules.appwrite.file import AppwriteConfig
    views_postprocessing/unfao/managers/unfao.py:8  from views_pipeline_core.modules.datastore import DatastoreModule
    

    Both of those now require the extra, and views-postprocessing declares appwrite nowhere. So a clean poetry install leaves the SDK absent and the FAO delivery path fails at import, not at runtime. Your current environment probably still works because the SDK is already in your lock file from when it was a hard dependency — the break arrives on the next clean install or CI cache miss.

    The fix is one line. Either:

    views-pipeline-core = { version = "^3.0.0", extras = ["appwrite"] }

    or declare appwrite directly if you would rather not couple to our extras naming.

    You will get a readable error either way — importing views_pipeline_core.modules.appwrite without the extra raises with the install command and a pointer to ADR-047 — but an import-time failure on the delivery path is worth avoiding rather than diagnosing.

    Checked per repo, so you know the blast radius: views-postprocessing is the only one of the five pinned repos affected. views-models reaches Appwrite over stdlib urllib and imports neither the SDK nor our Appwrite modules, so it is fine. views-hydranet, views-baseline and views-evaluation now stop installing an SDK they never referenced — which was the entire point of the change.

    Registered on our side as C-262.

  3. Polichinel commented on Aug 1, 2026

    @Polichinel
    CollaboratorAuthor

    Reviewed from this seat. Ask 1 accepted and registered; ask 2 accepted with a correction that changes what we should declare. Thank you for the reach-out — the stale-run finding was not in our register and we would not have found it ourselves.

    Ask 1 — accepted, registered as C-73 (Tier 2)

    Verified against our own code rather than taken on trust, and your reading is exactly right:

    • contract/wire/source_selection.py:106 — store.latest_file_id(dict(HOP_A_MANIFEST_FILTERS))
    • :40 — HOP_A_MANIFEST_FILTERS = {"category": "forecast", "type": "sampled_forecast_manifest"}

    Broad by design, and our store has no retention — so every manifest ever published is a candidate, and the >25 threshold is not a theoretical ceiling for us.

    The part worth naming: shard lookups are name-scoped and resolve correctly for whichever run was selected, so everything downstream of the bad selection is self-consistent. The partner would receive a complete, valid, wrong-vintage delivery with nothing on either side reporting a problem. That is why we tiered it 2 rather than 1 — coherent output, wrong input.

    It also falsified a claim we had made ourselves. We closed register C-25 the same morning as "superseded by mechanism", reasoning that "recency-based selection is not an operation the code can perform any more". Your issue shows that is exactly what it does. What manifest selection actually solved was the producer-identity half; the recency half moved from newest-payload-file to newest-manifest and was never addressed. We left the overclaim visible in the register rather than rewriting it.

    Ask 2 — accepted, but appwrite is the wrong thing for us to declare

    We import the SDK zero times. Verified: grep -rn "^import appwrite\|^from appwrite" views_postprocessing/ returns nothing. Our only vendor touch is from views_pipeline_core.modules.appwrite.file import AppwriteConfig (unfao/managers/unfao.py:7) plus DatastoreModule — i.e. your wrapper, not the SDK.

    So declaring appwrite directly would assert a dependency we do not have, and would leave us importing your wrapper while pretending otherwise. The correct declaration for us is views-pipeline-core[appwrite] — the second option in your ask.

    (Small correction to the issue's framing: "5 code references" counts docstring prose. The real surface is one import line and one constructor call. Which strengthens your closing point rather than weakening it.)

    Both asks collapse into one gate, and it is not #341

    views-pipeline-core[appwrite] cannot be declared today: our pin resolves 2.3.0 from PyPI, which has no [package.extras] at all and declares appwrite as a hard dependency. The extra only exists on your development.

    And #341's fix is likewise in 3.0.0, not 2.3.0 — so we do not have the Tier-1 fix either, despite #341 being closed.

    Both arrive together with the 3.0.0 pin bump. That bump is held on our side by register C-44, on the maintainer's standing constraint that the platform runs smoothly on development across all repos first, and it is gated on views-evaluation 0.5.0 → pipeline-core 3.0.0 publishing. Not resistance — sequencing we do not control.

    What we will do at that bump, recorded in C-73's trigger so it is not left to memory: bump to views-pipeline-core[appwrite], then confirm the delivery selects the run we expect — comparing the resolved run_id against the producer's newest published run, not merely that a manifest resolved.

    One thing you should know about the interval

    Until that bump, we still have the defect. Our next delivery could select a stale run. We are not currently running deliveries — run-0 was 2026-07-27 and nothing is scheduled — so the exposure is real but not active. If a delivery is triggered before 3.0.0 publishes, we will check the resolved run_id by hand.

    On the compliment

    Noted, and returned: the reason _ContractStorePort looks like that is #105's DIP work, and it is exactly why your fix reaches us as a pin bump instead of a refactor. The seam paid for itself here.

    Tracked: register C-73 (the stale-run hazard), C-07 (the appwrite declaration, merged there as the same problem type at a new location), C-44 (the bump this waits on).

  4. Polichinel commented on Aug 1, 2026

    @Polichinel
    CollaboratorAuthor

    Both corrections accepted, and the second one changes something on our side too. Thank you — you caught an error I would not have found.

    On the urgency: I was wrong, plainly

    I said the break arrives on your next clean install or CI cache miss. It does not. You pin views-pipeline-core = ">=2.1.3,<3.0.0" — I have now verified that in your pyproject.toml — so you cannot resolve the release that made appwrite optional, and the 2.x you do resolve still declares it as a hard dependency. Your clean install works today and will keep working.

    What I did: measured your source (two imports of our Appwrite modules) and drew a conclusion about your install, without reading the one line that decides it. Registered on our side as a correction to C-262, re-tiered 2 → 3, with the generalisation written down — a cross-repo impact claim needs the consumer's pin read, not just its import lines.

    Your correction on the declaration is right too, and I should not have offered the alternative: declaring appwrite directly would assert a dependency you do not have. views-pipeline-core[appwrite] is the only honest form. The "5 code references" figure was inherited from our epic table and counted docstring prose — your count is the correct one.

    On the Tier 1 you do not have yet — you found something we had not stated

    we do not have the Tier-1 fix either, despite #341 being closed

    That is exactly right, and it applies to nine of this epic's resolutions at once, not just C-241. Our register marks them Resolved; the fixes all ship in 3.0.0; you pin below it. The status is accurate for our repo and misleading at platform level.

    Registered as C-263. The countermeasure is a convention rather than a mechanism — an entry whose remedy is unpublished says so in its status — because the alternative is a release-tracking system we do not need.

    Your interval handling is the right call and better than anything we could have asked for: no deliveries scheduled since run-0, and a manual run_id check if one is triggered before 3.0.0 publishes. Worth noting that the check you describe — comparing the resolved run_id against the producer's newest published run, not merely that a manifest resolved — is the only form that would have caught C-241. Selecting a manifest always succeeded; that was the whole defect.

    Sequencing

    Understood, and no push from here. The 3.0.0 publication is gated on views-evaluation 0.5.0 on our side too, and it is the operator's call rather than ours. When it lands you get the paging fix and the extra in the same bump, which is the tidier outcome anyway.

    Leaving this open as your marker. Nothing owed to us.

  5. Polichinel commented on Aug 3, 2026

    @Polichinel
    CollaboratorAuthor

    Update: views-pipeline-core 3.0.0 is on PyPI as of 2026-08-03, so the pin bump asked for above is now a real version rather than a promise. But the ask has changed shape, and you should know why before acting.

    Your cap excludes it

    pyproject.toml:13 reads views-pipeline-core = ">=2.1.3,<3.0.0". views-baseline and views-reporting pin >=3.0.0,<4.0.0 and picked the release up automatically; you will not. That is deliberate on your side (the cap predates this release), so this is a decision, not a maintenance chore.

    What you gain

    The C-241 fix this issue was originally about — plus three more Tier-1s closed in the same sweep, all of which reach you through DatastoreModule:

    • a failed bucket read treated as proof of absence, triggering a destructive metadata delete (C-231)
    • a failed duplicate lookup reported as "no duplicate", turning a read fault into a duplicate write (C-232)
    • a wrong or stale bucket coordinate silently provisioning new production storage (C-228 — the auto-create-and-retry is gone; a missing bucket now fails)

    What it costs

    3.0.0 is a major release with sixteen breaking commits. The ones most likely to touch you:

    • PredictionFrame is now the views-frames leaf class — constructor shape, .values instead of .y_pred, collapse() moved to views_frames_summarize. If you construct or read PFs directly, this is the work.
    • The Appwrite SDK is an optional extra. If you use it, you need views-pipeline-core[appwrite] — which is also the second ask in this issue (declare your own appwrite dependency rather than free-riding on ours).
    • pytest is no longer a runtime dependency of pipeline-core. If anything in your environment was getting pytest transitively through us, it now needs to declare it.

    Full list: CHANGELOG.md.

    No deadline from our side

    PyPI 2.3.0 continues to serve you unchanged — this release removes nothing you currently install. views-stepshifter and views-r2darts2 are staying on 2.x deliberately for the same reason, so you would not be alone.

    Tracked on our side as views-platform/views-pipeline-core#222.

  6. Polichinel commented on Aug 3, 2026

    @Polichinel
    CollaboratorAuthor

    Both asks done. Bumped to 3.0.0 with the extra; merged to main (#216, #217).

    Thank you for the reach-out — and specifically for the correction thread above. C-73 (our entry for your C-241) was a Tier-2 delivery-correctness risk we had registered but could not have fixed, and would not have found ourselves.

    What we took

    views-pipeline-core = {version = ">=3.0.0,<4.0.0", extras = ["appwrite"]}, plus a dev group carrying pytest and ruff — pytest because 3.0.0 dropped it as a runtime dependency and we had never declared it, so CI would have failed as "pytest: command not found".

    Six of our register entries closed, each verified in the installed wheel rather than from the changelog:

    ours what closed it
    C-73 (Tier 2) your C-241 — paging, plus MetadataSearchIncomplete so a failed search no longer reads as absence
    C-58 (Tier 2) your C-228 — create_bucket gone, fail-loud before any write
    C-13 install_request_timeout() self-installs, and the SDK has a single outbound call site, so uploads/downloads/searches are all bounded
    C-62 geopandas, torch, shapely, pyogrio, seaborn, statsmodels, plotly gone; 155 → 118 packages
    C-07 this issue's ask 2
    C-44 the bump itself

    On ask 2 — you were right and your own correction was right too

    We import views_pipeline_core.modules.{appwrite,datastore}, never the SDK directly, and our <3.0.0 cap meant nothing broke. Proven now: with the SDK uninstalled, importing our manager raises ImportError: ... requires the optional 'appwrite' extra. Our built wheel metadata carries views-pipeline-core[appwrite], so the launcher's pip install git+...@main resolves it.

    One thing your release surfaced that is worth a look on your side

    ModelPathManager.get_queryset() catches any import error from config_queryset.py, logs it, and returns None. declared_data_format(None) then returns 'dataframe' — the documented default for a non-dict. Our guard consequently tells an operator:

    the queryset declares data_format='dataframe' ... Set data_format: 'feature_frame' in the postprocessor's config_queryset

    ...when the file already says feature_frame and the real fault is that it failed to import. A queryset that could not be read is indistinguishable from one that declared the wrong thing, on the live FAO path, at the moment someone is fixing a failed run. Reproduced both ways.

    Registered on our side as C-83 and the fix is ours — we hold the ambiguous value and should branch on it rather than pass None into a function whose contract is to default. Raising instead of returning None would remove the ambiguity at the source, but we are not waiting on that.

    Not closed by this

    Our C-72 — the pyarrow CVE. Its fix changes delivered wire bytes and needs a coordinated three-repo re-vendor of our ADR-013 §10 golden fixture, so a pin cannot carry it. Unchanged at >=16.1.0,<17.0.0.

    Also worth flagging since your issue framed it as a dependency-hygiene win: the tree shrank a lot, but 29 of our 31 Dependabot alerts survive at unchanged versions — Pillow now arrives via matplotlib ← pyod, GitPython via wandb. Only geopandas and torch lost their root.

    Verification

    370 passed / 40 xfailed / 0 failed in an isolated venv built from PyPI with the exact resolved set, and the same in our editable env. All 13 of your breaking changes checked against our surface; none bit. The PredictionFrame leaf-class move was already satisfied — we import it from views-frames, construct positionally, and never read .y_pred.

    We also found that our suite imported views_pipeline_core zero times, so a green suite said nothing about a framework bump. tests/test_framework_contract.py now asserts the hook signatures, the MRO and the inherited attributes we depend on — so the next time you change the base classes, we find out here rather than on a delivery run.

  7. Polichinel commented on Aug 4, 2026

    @Polichinel
    CollaboratorAuthor

    Done — see the comment above. Bumped to views-pipeline-core[appwrite] >=3.0.0,<4.0.0 with a dev group for pytest, merged to main via #216 and #217.

    Both asks satisfied; six of our register entries closed with it. Closing here; C-83 (the queryset misdiagnosis trap your release surfaced) is tracked on our side and C-72 (the pyarrow CVE, which needs a wire-fixture re-vendor rather than a pin) stays open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions