mirror of
https://github.com/kuhyx/diet-guard.git
synced 2026-11-10 21:27:11 +01:00
The button ran a full sync tick inline on the Tk thread, so the fullscreen lock was dead for its whole duration -- ~18-27s measured against the live remote, and unbounded in the worst case. Two changes, and the second is what makes the first cheap: * the pull moves to a daemon worker thread, with the result marshalled back through a queue polled on the Tk thread. The worker touches no widget, by construction: Tcl is not thread-safe and that includes `after`. Polling also makes the completion path testable with no thread at all. * the button now runs the *narrow* pull. Its question is identical to the gate's -- "did a peer log this slot?" -- and `_reconcile_after_fetch` reads only `due_slots()`, so the budget and food banks it used to fetch were never consulted here. The full tick still runs after the window closes. Measured 84ms against the live remote. Narrowing also shrinks the write race from tens of seconds to imperceptible: the pull persists its merge, so a meal logged mid-fetch could be overwritten by a snapshot taken before it. A second click while a fetch is in flight is now ignored, which is the reentrancy guard and the race fix in one. That guard started out as a disabled button, which is how it surfaced a latent bug worth recording: the fetch button widget is never stored on `GateWidgets`, so `self._widgets.fetch_button` did not exist and the disable was a silent no-op -- hidden because the suppression tuple included `AttributeError`. Coverage would have reported 100% on a guard that did nothing. Guarding on `_fetch_result` needs no widget at all. Also adds `INTERACTIVE_TIMEOUT_SECONDS = 2.0` for the paths a user waits on. `SYNC_TIMEOUT_SECONDS` only ever reached GitHub: crdt_sync v0.6.0's `firebase_client_for` takes no timeout, so Firebase used its 15s default and one hung session refresh stalled the lock window -- the journal has five such refresh failures in five days. Verified to fail closed to the local decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LJ5YnA3CERitQ57HjMu2Qz
168 lines
6.2 KiB
Python
168 lines
6.2 KiB
Python
"""Choosing and building the remote store one sync tick will use.
|
|
|
|
Split out of ``_sync.py`` for file size.
|
|
|
|
Two backends, and the tick needs exactly one: Firebase (primary, configured
|
|
at ``~/.config/crdt-sync/``) or a GitHub PAT mirror at
|
|
``~/.config/diet_guard/sync_token``. Neither configured is not an error --
|
|
:func:`_client_for_run` raises :class:`~diet_guard._sync.SyncError` and the
|
|
caller logs ``sync not configured`` and no-ops, which is what a machine that
|
|
has never been set up should do on a 15-minute timer.
|
|
|
|
``ConfigError`` subclasses ``Exception`` directly rather than
|
|
``RemoteSyncError``, so it is caught explicitly here and translated -- a
|
|
sibling-not-subclass relationship that has bitten this file before.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from typing import TYPE_CHECKING
|
|
|
|
from crdt_sync import (
|
|
CONFIG_FILE,
|
|
ConfigError,
|
|
FirebaseAuthError,
|
|
GitHubSyncClient,
|
|
RemoteSyncError,
|
|
firebase_client_for,
|
|
mirror_client_for,
|
|
)
|
|
|
|
from diet_guard._constants import (
|
|
INTERACTIVE_TIMEOUT_SECONDS,
|
|
SYNC_REPO_NAME,
|
|
SYNC_REPO_OWNER,
|
|
SYNC_TIMEOUT_SECONDS,
|
|
SYNC_TOKEN_FILE,
|
|
)
|
|
from diet_guard._sync_errors import SyncError
|
|
|
|
if TYPE_CHECKING:
|
|
from crdt_sync import RemoteStore
|
|
|
|
_logger = logging.getLogger(__name__)
|
|
|
|
|
|
def _remote_client(github: GitHubSyncClient) -> RemoteStore:
|
|
"""Return the backend to sync against.
|
|
|
|
Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a
|
|
mirror so a device that has not moved yet still converges; GitHub alone
|
|
otherwise. An unconfigured machine keeps syncing exactly as before.
|
|
|
|
Every sub-sync here (log, budget, food bank, manual bank) takes the client
|
|
as a parameter, so this one swap covers all of them.
|
|
|
|
The config file is checked before constructing anything, so an
|
|
unconfigured machine never reaches the network -- otherwise a suite that
|
|
blocks real sockets fails here rather than in the code under test.
|
|
|
|
Rolling back is deleting this function and passing ``github`` straight
|
|
through: no data moves either way.
|
|
"""
|
|
if not CONFIG_FILE.is_file():
|
|
return github
|
|
try:
|
|
return mirror_client_for("diet_guard", github)
|
|
except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc:
|
|
_logger.warning("Firebase unavailable, syncing via GitHub only: %s", exc)
|
|
return github
|
|
|
|
|
|
def _client_for_run() -> RemoteStore:
|
|
"""Return the backend for one sync run, requiring only *one* to be set up.
|
|
|
|
The PAT is no longer mandatory. Firebase is the primary backend now, and
|
|
requiring a GitHub token before ever constructing it meant a
|
|
Firebase-configured machine with no PAT could not sync at all -- the
|
|
commits that fixed exactly this ("Auto-sync on a Firebase-only device")
|
|
changed only the Dart side, leaving the Python half behind.
|
|
|
|
A missing PAT is therefore fatal only when Firebase is *also* unconfigured,
|
|
which is the genuine "nothing is set up" case the caller must report.
|
|
"""
|
|
try:
|
|
token = _read_token()
|
|
except SyncError:
|
|
if not CONFIG_FILE.is_file():
|
|
raise
|
|
# Firebase-only: there is no GitHub client to mirror to, so use the
|
|
# primary directly rather than failing the whole run.
|
|
#
|
|
# ConfigError is translated rather than propagated: it is *not* a
|
|
# RemoteSyncError (it subclasses Exception directly), so letting it out
|
|
# would escape every caller's catch tuple and raise a traceback out of
|
|
# the gate's fail-closed "Fetch from sync" button -- the exact failure
|
|
# the RemoteSyncError swap was made to stop. A config file that exists
|
|
# but is unusable is reached by precisely this branch.
|
|
try:
|
|
return firebase_client_for("diet_guard")
|
|
except ConfigError as exc:
|
|
message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}"
|
|
raise SyncError(message) from exc
|
|
return _remote_client(
|
|
GitHubSyncClient(
|
|
SYNC_REPO_OWNER,
|
|
SYNC_REPO_NAME,
|
|
token,
|
|
timeout_seconds=SYNC_TIMEOUT_SECONDS,
|
|
)
|
|
)
|
|
|
|
|
|
def _read_token() -> str:
|
|
"""Return the saved sync PAT, stripped of trailing whitespace.
|
|
|
|
Raises:
|
|
SyncError: If the token file is missing or empty -- the user has not
|
|
completed the one-time github.com setup step yet.
|
|
"""
|
|
if not SYNC_TOKEN_FILE.exists():
|
|
message = (
|
|
f"no sync token at {SYNC_TOKEN_FILE} -- create a fine-grained "
|
|
"GitHub PAT scoped to the syncs repo's contents and "
|
|
f"save it there (mode 600), then re-run sync"
|
|
)
|
|
raise SyncError(message)
|
|
token = SYNC_TOKEN_FILE.read_text().strip()
|
|
if not token:
|
|
msg = f"{SYNC_TOKEN_FILE} is empty"
|
|
raise SyncError(msg)
|
|
return token
|
|
|
|
|
|
def _apply_timeout(client: RemoteStore, seconds: float) -> None:
|
|
"""Tighten ``client``'s per-request budget, including any mirrored halves.
|
|
|
|
Reaches for the private ``_timeout_seconds`` because ``crdt_sync`` v0.6.0
|
|
exposes no other lever: ``firebase_client_for`` accepts no timeout, so a
|
|
Firebase client silently uses its 15s default and one hung request stalls
|
|
a path the user is watching. The attribute is read fresh on every request,
|
|
so assigning it here is enough.
|
|
|
|
Set via ``vars()`` rather than ``setattr`` so a client that does not carry
|
|
the attribute is left alone instead of gaining a meaningless one. The clean
|
|
fix is a ``timeout_seconds`` kwarg upstream; until that ships this keeps the
|
|
interactive paths bounded.
|
|
"""
|
|
for target in (
|
|
client,
|
|
getattr(client, "primary", None),
|
|
getattr(client, "mirror", None),
|
|
):
|
|
if target is not None and "_timeout_seconds" in vars(target):
|
|
vars(target)["_timeout_seconds"] = seconds
|
|
|
|
|
|
def _client_for_interactive_run() -> RemoteStore:
|
|
"""Return a sync client budgeted for a path the user is waiting on.
|
|
|
|
Same backends as :func:`_client_for_run`, but with
|
|
:data:`~diet_guard._constants.INTERACTIVE_TIMEOUT_SECONDS` in place of the
|
|
background tick's much longer budget.
|
|
"""
|
|
client = _client_for_run()
|
|
_apply_timeout(client, INTERACTIVE_TIMEOUT_SECONDS)
|
|
return client
|