promote, drift-check: a copy that caught up to an unrecorded canonical version, or in its own asset-path spelling, is refused as a collision and reported as drift
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 38/100
Direzione di ricerca
Start with the reproduction tests in tools/tests/test_promote_collisions.py, then read the stale checks in tools/promote and tools/drift-check (notably the referenced lines around 718–765 and 377–411). Run the tools/tests harness and use the two reproductions to verify both unrecorded canonical versions and asset-path-normalised copies are treated as stale rather than edited; the issue notes that version-lookup tests will need a fake.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Follow-up from the review of https://github.com/QuantEcon/lectures/pull/14. Line numbers are at #14's head as it merges, with #17 included.
What happens
Since 7128fb1 (in #14), a recorded source that no longer matches the canonical copy counts as stale, not edited, only if its raw digest equals one of two recorded digests: its own, or the canonical copy's at the last promotion (resolve_sources, tools/promote:718-726; check_lecture, tools/drift-check:377 and :394). Two kinds of copy that have only caught up fall outside those two digests:
- (a) A copy at a canonical version that was never recorded. In the nightly cycle the ledger records a canonical version only when a refresh PR is merged, and
drift/refreshis rebuilt from main on every run. If the canonical copy changes twice before that merge, or a version comes and goes between two nightly syncs, a series that ports the version in between holds a digest the ledger never saw. - (b) A normalised-equal source that caught up in its own asset-path spelling. Since #14, promote admits a source that equals the canonical copy once asset references are normalised, and records the source's own raw digest. When that copy catches up with the canonical version recorded at the last refresh, it keeps its own spelling, so its digest cannot equal the canonical copy's recorded digest.
In both cases, once the canonical copy has moved on:
promote --refreshrefuses the entry as a collision, and the nightly run goes red. Its message (tools/promote:761-765) says to "name the canonical series in sync/canonical.yml"; doing that makes the entryinterim, with the lagging copy recorded as a divergent lecture.drift-checkreportsdivergedand opens a drift issue saying the copy "has since been edited on its own", although it is only behind, not edited.
Both persist until the lagging copy reaches the canonical copy's current version. The same steps go through as stale-copy when the version was recorded (the v2 refresh merged before v3) or the spelling is identical; the latter is test_a_copy_that_caught_up_then_fell_behind_again_is_still_stale in tools/tests/test_promote_collisions.py.
What #17 changes
PR #17, which lands with #14, makes drift-check compare a moved source with the canonical copy's current text under promote's normalised_text (same_lecture, tools/drift-check:385). That covers the lockstep case in #15, but not the stale rule: tools/drift-check:394 still compares the copy's raw digest with the canonical copy's recorded raw digest, and promote is unchanged. With #17, the copy in (b) is reported as converged ... up to asset path spelling while the canonical copy stays at the version it caught up with, then as diverged once the canonical copy moves on. (a) is the same with or without #17.
Related to #15, which this extends: there, drift-check alone raised a false diverged that cleared once the refresh PR merged. Here promote refuses too, so no refresh PR clears it.
Before #14
On main, drift-check already reported (a) as diverged, since it had no caught-up rule, but the refresh went through (marking the entry interim) and the run stayed green. (b) cannot arise on main, which admitted only byte-identical sources.
Reproduction (with the tools/tests harness)
from conftest import lecture_text
FIG = "_static/lecture_specific/short_path/graph.png"
def test_a_copy_at_an_unrecorded_canonical_version_is_called_an_edit(make_repo):
repo = make_repo(["programming", "intermediate"])
for series in ("programming", "intermediate"):
repo.lecture(series, "pandas_panel", lecture_text("Panel Data", "Version one."))
assert repo.promote("pandas_panel").returncode == 0 # both recorded at v1
# programming publishes v2 and then v3 before a refresh PR is merged, so
# the ledger still records v1. intermediate ports v2, exactly.
repo.lecture("programming", "pandas_panel", lecture_text("Panel Data", "Version three."))
repo.move_pin("programming", 3)
repo.lecture("intermediate", "pandas_panel", lecture_text("Panel Data", "Version two."))
repo.move_pin("intermediate")
assert "[diverged] lectures/pandas_panel.md" in repo.drift_check().stdout
res = repo.promote("--refresh")
assert res.returncode == 1
assert "[pandas_panel] FAILED: pandas_panel.md differs across series" in res.stderr
# Following the advice in that message makes the entry interim.
repo.write_canonical_map({"pandas_panel": "programming"})
assert repo.promote("--refresh").returncode == 0
assert repo.entry("pandas_panel")["interim"] is True
def test_a_normalised_equal_copy_that_caught_up_is_called_an_edit(make_repo):
repo = make_repo(["intro", "intermediate"])
def publish(series, body): # one lecture: intro writes /_static/..., intermediate _static/...
repo.mirror_file(series, FIG, b"png")
ref = "/" + FIG if series == "intro" else FIG
repo.lecture(series, "short_path", lecture_text("Shortest Paths", body, refs=[ref]))
publish("intro", "Version one.")
publish("intermediate", "Version one.")
assert repo.promote("short_path").returncode == 0
publish("intro", "Version two.")
repo.move_pin("intro", 2)
assert repo.promote("--refresh").returncode == 0 # intermediate kept as a stale source
publish("intermediate", "Version two.") # caught up, in its own spelling
repo.move_pin("intermediate", 2)
publish("intro", "Version three.") # intro moves on before a refresh records the catch-up
repo.move_pin("intro", 3)
assert "[diverged] lectures/short_path.md" in repo.drift_check().stdout
res = repo.promote("--refresh")
assert res.returncode == 1
assert "[short_path] FAILED: short_path.md differs across series" in res.stderr
Both tests pass on #14's head, with #17 included (and at 7128fb1 before it), and the asserted steps print the same lines throughout:
[diverged] lectures/pandas_panel.md: the intermediate copy lectures/pandas_panel.md no longer matches its canonical home in programming
[pandas_panel] FAILED: pandas_panel.md differs across series (same: programming:91ab2fd94f58; different: intermediate:9edfc192d91e) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml
[diverged] lectures/short_path.md: the intermediate copy lectures/short_path.md no longer matches its canonical home in intro
[short_path] FAILED: short_path.md differs across series (same: intro:d9aec721be1c; different: intermediate:0deebc23b47c) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml
A drift-check run between intermediate's catch-up and intro's next edit reports [converged] lectures/short_path.md: the intermediate copy changed and now matches intro up to asset path spelling since #17; before #17 it reported [diverged] there too (the #15 case).
When it bites
Not today. Every non-canonical source in the committed ledger is a dp-test copy (25 entries), byte-identical to its canonical copy, and the sandbox is only edited deliberately, to exercise the tools. Apart from those, and two housekeeping pages that are not in the pool (status, zreferences), no lecture on the real mirror at the current pins is held identically by two canonical-eligible series, byte for byte or after normalisation.
(a) becomes possible with the two merges in QuantEcon/project-monorepo#76: pandas_panel (programming, which is canonical, and intermediate) and autodiff (programming and jax, once jax leaves PROMOTE_EXCLUDE) are to be held identically by two series and kept in step by hand. It then takes one port of a version that was never recorded: the canonical copy edited twice in one day, or edited again while the refresh PR for the first edit is still open.
(b) also needs the pair to spell an asset reference differently. Both copies of each pair spell theirs the same way today, but the acceptance criterion in QuantEcon/project-monorepo#76 only asks for the copies to be identical after the ledger's path normalisation. The first candidates that #15 names, short_path and ifp_egm, are not candidates: on the real mirror their jax copies are different lectures, which that item renames.
Suggested fix
Make "stale" mean "the same lecture as a version the canonical copy has had", with one rule shared by both tools, as #17 did for converged:
- For (b), compare in the form promotion uses. For example, on an entry with a normalised-equal source, record each source's normalised digest (sha256 of
normalised_text) beside its rawdigest, and apply the stale test to normalised digests; other entries, which today means all of them, re-dump unchanged. drift-check, which importsnormalised_textsince #17, applies the same test at:394. - For (a), also accept a copy that equals, up to normalisation, any version the canonical file had between its recorded
promoted_atpin and the current pin. drift-check already fetches the canonical copy by pin (fetch_at_pin), and GitHub's commits API filtered by path lists the versions in between. promote reads only the mirror today, so it would need the same best-effort lookup. - Until 2 lands, reword the refusal (
tools/promote:761-765) and thedivergeddetail (tools/drift-check:406-411): when the canonical copy has moved since promotion, a copy at no recorded version may be a port that is behind rather than an edit, and async/canonical.ymlline is the wrong remedy for it.
The two tests above, with their assertions inverted, cover the change; for (a) the harness needs a fake of the version lookup, since it cannot reach GitHub.
- Lingua principale
- Python
- Stelle
- 1
- Fork
- 0
- Merge medio
- 1g 3h
- PR unite (30g)
- 17
Preparare l'ambiente
Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di QuantEcon/lectures
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno
-
drift
Difficoltà 2/5 1-3 ore Idoneità per principianti 35/100
I maintainer di solito rispondono entro 1 giorno
-
question
Difficoltà 4/5 1-2 giorni Idoneità per principianti 44/100
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 58/100
QuantEcon/lectures#40 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di QuantEcon/lectures
Issue simili
-
Edit: TVS Encarnacion [PY]Apertachannels:edit check:passed
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
I maintainer di solito rispondono entro 1 giorno
-
[Bug] The blue-green mitigation strategy lists catalog names and views before the source tablesAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
aws-samples/sample-aws-genai-db-modernizer#2097 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
enhancement
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
ietf-tools/datatracker#12027 ·
I maintainer di solito rispondono entro 1 giorno
-
[Bug]: Python SDK overwrites OVPack backups with HTTP 200 JSON errorsForse già presa @stefanielizeth73-art l’ha presa oggi. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
volcengine/OpenViking#7347 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
deepset-ai/haystack-core-integrations#4228 ·
I maintainer di solito rispondono entro 1 giorno