mirror of
https://github.com/brunopostle/ifcurl.git
synced 2026-07-12 10:18:14 +00:00
git.py: log warning and return is_stale flag when mutable-ref fetch fails
When a branch/HEAD fetch fails (network error, auth failure, server down), _open_remote now logs a warning rather than silently passing. A boolean is_stale propagates through _get_repo → fetch_ifc so callers know the data comes from a stale local cache. The service adds X-Ifcurl-Cache: stale to /preview and /render_diff responses when stale data was served, so Forgejo or a browser viewer can surface a warning to the user. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
a6206a459e
commit
6e62f9e51c
5 changed files with 72 additions and 34 deletions
|
|
@ -35,10 +35,13 @@ from __future__ import annotations
|
|||
import hashlib
|
||||
|
||||
# allows git import even if git executable isn't found during import
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
from platformdirs import user_cache_dir
|
||||
|
||||
os.environ.setdefault("GIT_PYTHON_REFRESH", "quiet")
|
||||
|
|
@ -53,12 +56,16 @@ except ImportError:
|
|||
from ifcurl.url import IfcUrl
|
||||
|
||||
|
||||
def fetch_ifc(ifc_url: IfcUrl, token: str | None = None) -> tuple[str, bytes]:
|
||||
"""Return ``(commit_hexsha, ifc_bytes)`` for the file addressed by *ifc_url*.
|
||||
def fetch_ifc(ifc_url: IfcUrl, token: str | None = None) -> tuple[str, bytes, bool]:
|
||||
"""Return ``(commit_hexsha, ifc_bytes, is_stale)`` for the file addressed by *ifc_url*.
|
||||
|
||||
The commit hexsha is the resolved, immutable identifier for the ref —
|
||||
useful as a cache key even when the URL uses a mutable ref like a branch.
|
||||
|
||||
*is_stale* is ``True`` when the URL uses a mutable ref (branch/HEAD) and
|
||||
the remote fetch failed — the returned bytes come from the local cache and
|
||||
may not reflect the latest commit. A warning is logged in this case.
|
||||
|
||||
:param ifc_url: A parsed :class:`IfcUrl`.
|
||||
:param token: Optional bearer token for HTTPS authentication. Injected
|
||||
as ``https://<token>@host/path``. Ignored for SSH and local repos.
|
||||
|
|
@ -73,8 +80,9 @@ def fetch_ifc(ifc_url: IfcUrl, token: str | None = None) -> tuple[str, bytes]:
|
|||
if ifc_url.path is None:
|
||||
raise ValueError("URL has no 'path' parameter — cannot fetch IFC file")
|
||||
|
||||
repo = _get_repo(ifc_url, token=token)
|
||||
return _read_commit_blob(repo, ifc_url.git_ref(), ifc_url.path)
|
||||
repo, is_stale = _get_repo(ifc_url, token=token)
|
||||
hexsha, data = _read_commit_blob(repo, ifc_url.git_ref(), ifc_url.path)
|
||||
return hexsha, data, is_stale
|
||||
|
||||
|
||||
def diff_text(base_url: IfcUrl, head_url: IfcUrl, token: str | None = None) -> str:
|
||||
|
|
@ -125,7 +133,7 @@ def fetch_ifc_bytes(ifc_url: IfcUrl, token: str | None = None) -> bytes:
|
|||
:raises ValueError: If ``ifc_url.path`` is unset, the repo cannot be
|
||||
reached, or the file is not found at the specified ref.
|
||||
"""
|
||||
_, data = fetch_ifc(ifc_url, token=token)
|
||||
_, data, _ = fetch_ifc(ifc_url, token=token)
|
||||
return data
|
||||
|
||||
|
||||
|
|
@ -134,10 +142,10 @@ def fetch_ifc_bytes(ifc_url: IfcUrl, token: str | None = None) -> bytes:
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _get_repo(ifc_url: IfcUrl, token: str | None = None) -> git.Repo:
|
||||
"""Open or fetch the repository for *ifc_url*."""
|
||||
def _get_repo(ifc_url: IfcUrl, token: str | None = None) -> tuple[git.Repo, bool]:
|
||||
"""Open or fetch the repository for *ifc_url*. Returns ``(repo, is_stale)``."""
|
||||
if ifc_url.transport == "local":
|
||||
return _open_local(ifc_url.repo_path)
|
||||
return _open_local(ifc_url.repo_path), False
|
||||
return _open_remote(ifc_url.git_remote_url(), ifc_url.is_mutable_ref(), token=token)
|
||||
|
||||
|
||||
|
|
@ -272,13 +280,17 @@ def _clone_bare(auth_url: str, git_dir: Path, remote_url: str) -> git.Repo:
|
|||
|
||||
def _open_remote(
|
||||
remote_url: str, is_mutable: bool, token: str | None = None
|
||||
) -> git.Repo:
|
||||
"""Return a GitPython Repo for *remote_url*, cloning it if necessary.
|
||||
) -> tuple[git.Repo, bool]:
|
||||
"""Return ``(repo, is_stale)`` for *remote_url*, cloning if necessary.
|
||||
|
||||
Bare clones are stored under the OS cache dir keyed on the clean URL.
|
||||
For mutable refs (branches, HEAD) the remote is fetched on every call.
|
||||
Immutable refs (commit hashes, tags) skip the fetch.
|
||||
|
||||
*is_stale* is ``True`` when a mutable-ref fetch was attempted but all
|
||||
network attempts failed — the caller receives cached data that may be
|
||||
outdated. A warning is logged in this case.
|
||||
|
||||
If *token* is provided it is injected into the URL for clone and fetch
|
||||
operations but is never written to the on-disk git config.
|
||||
|
||||
|
|
@ -288,6 +300,7 @@ def _open_remote(
|
|||
cache_dir = _cache_dir_for(remote_url)
|
||||
git_dir = cache_dir / "repo.git"
|
||||
auth = _auth_url(remote_url, token)
|
||||
is_stale = False
|
||||
|
||||
if not git_dir.exists():
|
||||
repo = _clone_bare(auth, git_dir, remote_url)
|
||||
|
|
@ -296,6 +309,7 @@ def _open_remote(
|
|||
else:
|
||||
repo = git.Repo(str(git_dir))
|
||||
if is_mutable:
|
||||
fetch_ok = False
|
||||
try:
|
||||
if auth != remote_url:
|
||||
# Fetch directly from the authenticated URL so the token
|
||||
|
|
@ -304,20 +318,29 @@ def _open_remote(
|
|||
fallback = _http_fallback(auth)
|
||||
try:
|
||||
repo.git.fetch(fetch_url, "+refs/*:refs/*")
|
||||
fetch_ok = True
|
||||
except git.exc.GitCommandError:
|
||||
if fallback:
|
||||
try:
|
||||
repo.git.fetch(fallback, "+refs/*:refs/*")
|
||||
fetch_ok = True
|
||||
except git.exc.GitCommandError:
|
||||
pass
|
||||
else:
|
||||
repo.git.fetch("origin")
|
||||
fetch_ok = True
|
||||
except git.exc.GitCommandError:
|
||||
pass # offline — use cached data
|
||||
pass # offline — fall through to stale check
|
||||
|
||||
if not fetch_ok:
|
||||
is_stale = True
|
||||
_logger.warning(
|
||||
"fetch failed for %r — serving stale cached data", remote_url
|
||||
)
|
||||
_evict_if_needed()
|
||||
|
||||
_touch_cache(cache_dir)
|
||||
return repo
|
||||
return repo, is_stale
|
||||
|
||||
|
||||
def _read_commit_blob(
|
||||
|
|
|
|||
|
|
@ -415,7 +415,7 @@ def preview(request: PreviewRequest) -> Response:
|
|||
|
||||
# --- Fetch IFC bytes + commit hexsha ---
|
||||
try:
|
||||
hexsha, ifc_bytes = fetch_ifc(ifc_url, token=token)
|
||||
hexsha, ifc_bytes, is_stale = fetch_ifc(ifc_url, token=token)
|
||||
except ImportError as exc:
|
||||
raise HTTPException(status_code=503, detail=str(exc)) from exc
|
||||
except ValueError as exc:
|
||||
|
|
@ -470,19 +470,23 @@ def preview(request: PreviewRequest) -> Response:
|
|||
_t3_put(hexsha, ifc_url.path, ifc_url.selector, new_guids)
|
||||
|
||||
# --- Tier 4 / 4m: store PNG and return with appropriate cache headers ---
|
||||
extra_headers = {"X-Ifcurl-Cache": "stale"} if is_stale else {}
|
||||
if ifc_url.is_mutable_ref():
|
||||
_t4m_put(request.url, png_bytes)
|
||||
return Response(
|
||||
content=png_bytes,
|
||||
media_type="image/png",
|
||||
headers={"Cache-Control": f"public, max-age={_T4M_TTL}"},
|
||||
headers={"Cache-Control": f"public, max-age={_T4M_TTL}", **extra_headers},
|
||||
)
|
||||
else:
|
||||
_t4_put(request.url, png_bytes)
|
||||
return Response(
|
||||
content=png_bytes,
|
||||
media_type="image/png",
|
||||
headers={"Cache-Control": "public, max-age=31536000, immutable"},
|
||||
headers={
|
||||
"Cache-Control": "public, max-age=31536000, immutable",
|
||||
**extra_headers,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -511,7 +515,7 @@ def bcf_export(request: BcfRequest) -> Response:
|
|||
if token is None and ifc_url.host:
|
||||
token = get_token_for_host(ifc_url.host)
|
||||
try:
|
||||
hexsha, ifc_bytes = fetch_ifc(ifc_url, token=token)
|
||||
hexsha, ifc_bytes, _ = fetch_ifc(ifc_url, token=token)
|
||||
except (ImportError, ValueError) as exc:
|
||||
raise HTTPException(status_code=404, detail=str(exc)) from exc
|
||||
|
||||
|
|
@ -614,8 +618,8 @@ def render_diff(request: DiffRequest) -> Response:
|
|||
|
||||
# --- Fetch IFC bytes for both commits ---
|
||||
try:
|
||||
base_hexsha, base_bytes = fetch_ifc(base_url, token=token)
|
||||
head_hexsha, head_bytes = fetch_ifc(head_url, token=token)
|
||||
base_hexsha, base_bytes, base_stale = fetch_ifc(base_url, token=token)
|
||||
head_hexsha, head_bytes, head_stale = fetch_ifc(head_url, token=token)
|
||||
except ImportError as exc:
|
||||
raise HTTPException(status_code=503, detail=str(exc)) from exc
|
||||
except ValueError as exc:
|
||||
|
|
@ -667,16 +671,22 @@ def render_diff(request: DiffRequest) -> Response:
|
|||
raise HTTPException(status_code=422, detail=str(exc)) from exc
|
||||
|
||||
# --- Tier 4: cache and return ---
|
||||
diff_stale_headers = (
|
||||
{"X-Ifcurl-Cache": "stale"} if (base_stale or head_stale) else {}
|
||||
)
|
||||
if both_immutable:
|
||||
_t4_put(cache_key, png_bytes)
|
||||
return Response(
|
||||
content=png_bytes,
|
||||
media_type="image/png",
|
||||
headers={"Cache-Control": "public, max-age=31536000, immutable"},
|
||||
headers={
|
||||
"Cache-Control": "public, max-age=31536000, immutable",
|
||||
**diff_stale_headers,
|
||||
},
|
||||
)
|
||||
|
||||
return Response(
|
||||
content=png_bytes,
|
||||
media_type="image/png",
|
||||
headers={"Cache-Control": "no-store"},
|
||||
headers={"Cache-Control": "no-store", **diff_stale_headers},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -206,7 +206,7 @@ class TestBcfEndpoint:
|
|||
ifc_bytes = model_with_geometry.to_string().encode()
|
||||
|
||||
def mock_fetch(ifc_url, token=None):
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
selector_url = MUTABLE_URL.replace(
|
||||
"?path=model.ifc", "?path=model.ifc&selector=IfcWall"
|
||||
|
|
|
|||
|
|
@ -18,31 +18,36 @@ def _local_url(repo_path: str, ref: str = "HEAD", path: str = "model.ifc") -> If
|
|||
class TestFetchIfc:
|
||||
def test_returns_hexsha_and_bytes(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"])
|
||||
hexsha, data = fetch_ifc(url)
|
||||
hexsha, data, _ = fetch_ifc(url)
|
||||
assert len(hexsha) == 40
|
||||
assert all(c in "0123456789abcdef" for c in hexsha)
|
||||
assert isinstance(data, bytes)
|
||||
assert len(data) > 0
|
||||
|
||||
def test_local_repo_never_stale(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"])
|
||||
_, _, is_stale = fetch_ifc(url)
|
||||
assert is_stale is False
|
||||
|
||||
def test_hexsha_matches_commit(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"])
|
||||
hexsha, _ = fetch_ifc(url)
|
||||
hexsha, _, _ = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
|
||||
def test_bytes_match_committed_file(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"])
|
||||
_, data = fetch_ifc(url)
|
||||
_, data, _ = fetch_ifc(url)
|
||||
assert data == local_ifc_repo["bytes"]
|
||||
|
||||
def test_explicit_commit_hash_ref(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"], ref=local_ifc_repo["hexsha"])
|
||||
hexsha, data = fetch_ifc(url)
|
||||
hexsha, data, _ = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
assert data == local_ifc_repo["bytes"]
|
||||
|
||||
def test_tag_ref(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"], ref="tags/v1.0")
|
||||
hexsha, data = fetch_ifc(url)
|
||||
hexsha, data, _ = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
|
||||
def test_branch_ref(self, local_ifc_repo):
|
||||
|
|
@ -50,7 +55,7 @@ class TestFetchIfc:
|
|||
|
||||
branch = gitpkg.Repo(local_ifc_repo["path"]).active_branch.name
|
||||
url = _local_url(local_ifc_repo["path"], ref=f"heads/{branch}")
|
||||
hexsha, data = fetch_ifc(url)
|
||||
hexsha, data, _ = fetch_ifc(url)
|
||||
assert hexsha == local_ifc_repo["hexsha"]
|
||||
assert data == local_ifc_repo["bytes"]
|
||||
|
||||
|
|
@ -85,7 +90,7 @@ class TestFetchIfcBytes:
|
|||
|
||||
def test_backwards_compat_with_fetch_ifc(self, local_ifc_repo):
|
||||
url = _local_url(local_ifc_repo["path"])
|
||||
_, expected = fetch_ifc(url)
|
||||
_, expected, _ = fetch_ifc(url)
|
||||
assert fetch_ifc_bytes(url) == expected
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ def tmp_t4_cache(tmp_path):
|
|||
|
||||
|
||||
def _mock_fetch(ifc_bytes: bytes):
|
||||
return lambda ifc_url, token=None: (FAKE_HEXSHA, ifc_bytes)
|
||||
return lambda ifc_url, token=None: (FAKE_HEXSHA, ifc_bytes, False)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -149,7 +149,7 @@ class TestTier2Cache:
|
|||
def counting_fetch(ifc_url, token=None):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
# Disable tier-4m so both requests reach fetch_ifc, letting us
|
||||
# verify that tier-2 serves ifc_bytes from cache on the second call.
|
||||
|
|
@ -305,7 +305,7 @@ class TestAuthentication:
|
|||
|
||||
def capturing_fetch(ifc_url, token=None):
|
||||
received_token.append(token)
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
with (
|
||||
patch("ifcurl.service.fetch_ifc", capturing_fetch),
|
||||
|
|
@ -323,7 +323,7 @@ class TestAuthentication:
|
|||
|
||||
def capturing_fetch(ifc_url, token=None):
|
||||
received_token.append(token)
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
monkeypatch.setattr(
|
||||
"ifcurl.service.get_token_for_host", lambda host: "config_token"
|
||||
|
|
@ -344,7 +344,7 @@ class TestAuthentication:
|
|||
|
||||
def capturing_fetch(ifc_url, token=None):
|
||||
received_token.append(token)
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
monkeypatch.setattr(
|
||||
"ifcurl.service.get_token_for_host", lambda host: "config_token"
|
||||
|
|
@ -389,7 +389,7 @@ class TestGetEndpoint:
|
|||
|
||||
def mock_fetch(ifc_url, token=None):
|
||||
received["token"] = token
|
||||
return FAKE_HEXSHA, ifc_bytes
|
||||
return FAKE_HEXSHA, ifc_bytes, False
|
||||
|
||||
with (
|
||||
patch("ifcurl.service.fetch_ifc", mock_fetch),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue