Backup folders on network shares often refuse SQLite's file locks (CIFS
without nobrl, NFS without lockd). SQLite reports a refused lock as
SQLITE_BUSY, so every backup to such a folder retried until the 60 second
give-up and blamed a locked database, while the live database was not
locked at all (#1246).
The target is a scratch .pending file that nothing else opens, so open it
with nolock=1. The give-up message now names the live database, since that
is the only file a backup can still wait on.
sqlite3's Connection.backup() retries forever while the source is busy,
calling progress with 0 of 0 pages each time. The backup ran synchronously
on the event loop, so a lock that did not clear froze every request and
spammed "Copied 0 of 0 pages." into the log (#1191).
Run backups in a worker thread, give up after the source has been locked
for 60 seconds, and serialize backups now that they can overlap. Per-page
progress drops to debug.
A sqlite3.Connection used as a context manager is a transaction manager: it
commits or rolls back on exit, it does not close the connection. Both handles
stayed open past the `with` block in Database.backup().
backup_and_rotate() then unlinks and moves the .pending file it just wrote.
POSIX allows unlinking a file with open handles, so this was invisible on
Linux, but on Windows it raises WinError 32 and breaks both the scheduled
midnight backup and the manual backup endpoint. It also leaked a pair of
connections per backup on every platform.
Wrap both connections in contextlib.closing instead. Connection.backup()
commits on the destination internally, so nothing is lost by dropping the
transaction-managing context manager.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Task 10: config.js JSON-encodes the base path, so a backslash cannot
escape the string literal the way the old " check allowed; search.py
escapes LIKE wildcards at both its call sites, using a helper moved into
database/utils.py so it and extra_field_query share one implementation.
Task 9 was a pair of decisions rather than fixes. /info keeps its paths:
both clients render them and removing response fields breaks API v1, while
the disclosure buys nothing against an API that is unauthenticated anyway.
/metrics keeps its shape but now says what it publishes when switched on.
Also pins the unit tests to a throwaway data directory -- importing
spoolman.main creates one as a side effect, and it was the real one.
Rotating deletes the oldest restore point and shifts the rest down, so enough
calls in quick succession leave nothing but snapshots of whatever state the
database is in now. POST /backup takes no parameters, so a bodyless cross-origin
form post reached it; the audit destroyed a full six-deep history that way
(todo.md task 2).
The origin guard from the previous commit already refuses that request, but the
rotation itself should not be this easy to abuse, so two further changes:
Rotation is skipped when the database has not actually changed. Not by comparing
the live database file, which the audit suggested: under WAL the live file does
not yet contain recent commits, so comparing it would skip backups that were
genuinely needed. Instead the snapshot is written to a pending file first and
compared against the newest backup -- sqlite's backup API reads through the
connection, so it sees WAL content. Identical means the pending file is
discarded and the history is left alone. This is fail-safe: if the comparison
ever came out unequal for an unchanged database, the result is the old behaviour
of rotating, never a missed backup.
Rotation is also rate-limited to one per five minutes, returning the newest
existing backup instead. Never applied when there is no backup to fall back on.
BackupResponse gained a `created` field so a client can tell the difference.
Additive, so v1-compatible, but load-bearing: telling a user they have a fresh
restore point when they do not is exactly the wrong failure mode here.
Verified against a real server -- six rapid calls now leave exactly one backup
and never rotate.
added env variable SPOOLMAN_METRICS_ENABLED for enabled database collector
set metrics path as /metrics
checked Null values at vendors, init weight and price
removed some logs
Donkie