Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pipeline/fetch.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ The OAuth `client_secret` also lives inside the volume — never in git.
| `DRIVE_FOLDER` / `DRIVE_FOLDER_ID` | — | what to mirror (define one; id wins) |
| `RAW_DIR` | `/data/raw` | mirror dir (raw-drive volume) |
| `POLL_INTERVAL_SECONDS` | `1800` | loop cadence (`--once` for a single pass) |
| `GOOGLE_DOCS_FORMAT` | `md` | export format for native Google Docs |
| `GOOGLE_DOCS_FORMAT` | `md` | export format for native Google Docs. Changing it re-exports the affected files on the next pass — the skip compares the recorded filename against the one the current config would write, not just the remote fingerprint (case-insensitively, so `MD` is not a change from `md`) |
| `GOG_ACCOUNT` | keyring default | gog account |
| `GOG_ALL_DRIVES` | `true` | include shared drives |
17 changes: 16 additions & 1 deletion pipeline/fetch/src/drive_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,22 @@ def sync_once(cfg: Config, folder_id: str) -> dict:
# rename-cleanup below never unlinks the freshly written sidecar.
if _clobbered_by_sidecar(cfg, fid, prev_local):
prev_local = None
if prev and prev.get("fingerprint") == fp and prev_local and (cfg.raw_dir / prev_local).exists():
# The skip must also require the recorded path to be the name THIS config would write.
# `fingerprint` is modifiedTime|size|md5 — purely remote — so a local export-format change
# (GOOGLE_DOCS_FORMAT md -> pdf) is invisible to it, and comparing only "some file is at the
# recorded path" meant the switch did nothing, forever: no download, no log, no error, while
# clean kept converting the old export.
#
# Case-INSENSITIVE, for the same reason content_name and _clobbered_by_sidecar are: on a
# case-folding filesystem (macOS APFS, Windows, Docker Desktop bind mounts) `D.md` and `D.MD`
# are one file, so a case-only difference is not a format change. Treating it as one is
# actively harmful here — the download writes `D.MD`, then the rename cleanup below unlinks
# `D.md`, which is the SAME INODE, so the mirror loses the document for a whole poll interval.
# `GOOGLE_DOCS_FORMAT=MD` (a typo in the very knob this fixes) is enough to trigger it.
_, expected_local = content_name(cfg, d, mime, fid)
if (prev and prev.get("fingerprint") == fp and prev_local
and prev_local.lower() == expected_local.lower()
and (cfg.raw_dir / prev_local).exists()):
# Backfill/refresh path metadata without redownloading unchanged files.
touched = False
for k, v in lineage.items():
Expand Down
68 changes: 68 additions & 0 deletions pipeline/fetch/tests/test_drive_fetch.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""drive_fetch with explicit Config — no global monkeypatching, no env juggling.
The gog CLI is always faked; no network, no binary."""
import dataclasses
import json

import drive_fetch as df
Expand Down Expand Up @@ -320,6 +321,73 @@ def test_mass_deletion_brake_counts_only_owned_entries(sync_env):
assert "slack-general-2026-01" in json.loads((tmp_path / "_state.json").read_text())["files"]


def test_changing_the_export_format_re_downloads(sync_env):
"""`GOOGLE_DOCS_FORMAT` is a documented knob (docs/pipeline/fetch.md, pipeline/.env.example), but
the skip decision only compared the remote fingerprint and "some file is at the recorded path" —
never whether that path is the name the CURRENT config would produce. `fingerprint` is
modifiedTime|size|md5, i.e. purely remote, so a LOCAL export-format change is invisible to it.

Switching md -> pdf therefore did nothing, forever: no download, no log line, no error. The
operator believes PDFs are flowing while clean keeps converting the old Markdown, and no recipe in
docs/operations/runbook.md tells you to wipe raw/ for this."""
cfg, tmp_path, fake = sync_env
fake.items.append({"id": "D", "name": "a doc", "modifiedTime": "t1", "parents": ["folder1"],
"mimeType": "application/vnd.google-apps.document"})
df.sync_once(cfg, "root")
manifest = json.loads((tmp_path / "_state.json").read_text())["files"]
assert manifest["D"]["localPath"] == "D.md"
assert (tmp_path / "D.md").exists()
fake.downloads.clear()

stats = df.sync_once(dataclasses.replace(cfg, docs_format="pdf"), "root")
assert "D" in fake.downloads, "a format change must re-export" # today: skipped, no download
assert (tmp_path / "D.pdf").exists()
manifest = json.loads((tmp_path / "_state.json").read_text())["files"]
assert manifest["D"]["localPath"] == "D.pdf"
assert not (tmp_path / "D.md").exists(), "the stale export must not linger in the mirror"
assert stats["errors"] == 0

# ...and it settles: the next pass with the same config re-downloads nothing
fake.downloads.clear()
stats2 = df.sync_once(dataclasses.replace(cfg, docs_format="pdf"), "root")
assert fake.downloads == [] and stats2["skipped"] == 3


def test_a_case_only_name_difference_is_not_a_format_change(sync_env):
"""The expected-name check must be case-INSENSITIVE, like content_name and
_clobbered_by_sidecar. On a case-folding filesystem `D.md` and `D.MD` are one file, so treating a
case-only difference as a format change is worse than missing it: the download writes `D.MD`, and
the rename cleanup then unlinks `D.md` — the SAME INODE — leaving the mirror without the document
for a whole poll interval. `GOOGLE_DOCS_FORMAT=MD`, a typo in the very knob this check exists for,
is enough to trigger it."""
cfg, tmp_path, fake = sync_env
fake.items.append({"id": "D", "name": "a doc", "modifiedTime": "t1", "parents": ["folder1"],
"mimeType": "application/vnd.google-apps.document"})
df.sync_once(cfg, "root")
assert (tmp_path / "D.md").exists()
fake.downloads.clear()

stats = df.sync_once(dataclasses.replace(cfg, docs_format="MD"), "root")
assert fake.downloads == [], "a case-only difference must not re-export"
assert stats["skipped"] == 3
# the decisive assertion: the document is still IN the mirror
assert (tmp_path / "D.md").exists()


def test_a_recorded_path_missing_from_disk_re_downloads(sync_env):
"""The `.exists()` half of the skip: a manifest entry whose file is gone must re-download. It
limits any name-comparison mistake to a single cycle instead of a permanent silent absence, so it
is load-bearing rather than belt-and-braces — and nothing pinned it."""
cfg, tmp_path, fake = sync_env
df.sync_once(cfg, "root")
fake.downloads.clear()
(tmp_path / "A.pdf").unlink() # the mirror lost the content, manifest still records it

stats = df.sync_once(cfg, "root")
assert fake.downloads == ["A"]
assert (tmp_path / "A.pdf").exists() and stats["skipped"] == 1


def test_sync_once_backfills_lineage_without_download(sync_env):
cfg, tmp_path, fake = sync_env
df.sync_once(cfg, "root")
Expand Down
Loading