Skip to content

fix(core): verify that store_entity actually created the page - #182

Open
LukasGold wants to merge 2 commits into
mainfrom
fix/175-store-entity-verify
Open

LukasGold wants to merge 2 commits into
mainfrom
fix/175-store-entity-verify

Conversation

@LukasGold

@LukasGold LukasGold commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Fixes #175.

Changes

  • New StoreEntityParam.verify_write, default True.
  • store_entity collects the titles it sent an edit for and queries their
    existence after the upload. A page that is absent is removed from
    StoreEntityResult.pages and added to StoreEntityResult.failed as a new
    OSW.PageNotCreatedError, which makes store_entity raise
    StoreEntityPartialError through the existing path.
  • New OSW._query_missing_page_titles(titles): one action=query&prop=info
    request per 50 titles, mapping normalized titles back to the requested form.
  • New OSW._get_missing_page_titles(titles, confirm_delay_s=5): queries a
    second time, 5 seconds later, for the titles the first query reported as
    missing.
  • A verification query that raises is caught and logged. The pages stay in
    StoreEntityResult.pages.
  • The comment claiming store_entity_ only records created_pages on success
    now says what actually happens.

Rationale

  • created_pages[page.title] = page runs for every page that reaches the last
    statement of store_entity_. That only means no exception was raised. It
    does not mean the page exists.
  • page.changed is no help: it was True in the reported failing case as
    well.
  • The query goes to the MediaWiki API directly and not through
    WtSite.get_page, because the page cache would answer with the state from
    before the write.
  • Batching by 50 titles keeps the cost at one extra request per 50 stored
    pages rather than one per page.
  • The second query prevents the opposite defect: a read answered by a database
    replica that does not have the write yet would otherwise fail a store that
    did apply. A page still absent seconds later is not explained by that lag.
  • A failed verification query is not evidence that the writes failed, and
    letting it propagate would discard the results the call had already
    collected. Reporting the pages and logging the error keeps more information
    than either alternative.
  • Reusing failed and StoreEntityPartialError means callers that already
    handle partial failures need no change, and result.stored stays truthful.
  • Verification is skipped when offline is True and for pages the overwrite
    policy kept unedited, since no write was sent for those.

Behaviour change

  • A store that silently did not apply now raises StoreEntityPartialError
    instead of returning normally. Callers that want the previous behaviour can
    pass verify_write=False.
  • Every online store_entity call that edits at least one page makes one
    additional API request per 50 edited pages, and waits 5 seconds plus one
    further request if a page is reported as missing.

Verification

  • 10 tests in tests/test_store_entity_verify.py cover the absent page, the
    mixed batch, both skip conditions, the confirmation query, a page that
    appears only on the second query, a failing query, the batching by 50 and the
    title normalization.
  • Full unit suite: 217 passed. tests/test_store_entity_failure.py and
    tests/test_overwrite_policy.py construct an OSW without a real site, so
    they now stub _get_missing_page_titles to report every page as existing.
    Their assertions are unchanged.
  • The added request count was not measured against a live instance. It is
    ceil(edited_pages / 50) per call by construction.
  • The server-side reason why such a write is rejected is still not identified.
    This change makes the library report the outcome correctly, it does not fix
    the write.

- add StoreEntityParam.verify_write, default True
- query the edited titles after the upload, one API request per 50 titles
- an absent page goes to StoreEntityResult.failed as PageNotCreatedError
  instead of being reported in StoreEntityResult.pages
- the query bypasses the page cache, which would answer pre-write
- correct the comment claiming created_pages only records successes

Closes #175
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v2.3.4 (current: v2.3.3).

Changelog preview (truncated)
## v2.3.4 (2026-09-18)

### Bug Fixes

- **core**: Confirm a missing page before reporting a failed store
  ([`5756436`](https://github.com/OpenSemanticLab/osw-python/commit/5756436e08ba95be3e177c6a78c424bfdba756c8))

- **core**: Verify that store_entity actually created the page
  ([`f84a529`](https://github.com/OpenSemanticLab/osw-python/commit/f84a5297c605fc458783e5497edeb1f97113053b))

### Chores

- Relicense to Apache-2.0 ([#173](https://github.com/OpenSemanticLab/osw-python/pull/173),
  [`848615c`](https://github.com/OpenSemanticLab/osw-python/commit/848615cbd1e06401922eda88231a51028440a242))

Preview via python-semantic-release and conventional commits.

- query a title reported as missing a second time, 5 s later, so a read
  from a lagging database replica cannot fail a store that applied
- a failing verification query no longer discards the results of the whole
  store_entity call: the pages stay reported and the error is logged
- add tests for both paths
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

store_entity reports success for a page it never created

1 participant