From a6974e24439e678d6667d97cf27ccb8031923174 Mon Sep 17 00:00:00 2001 From: louispaulb Date: Thu, 21 May 2026 15:15:31 -0400 Subject: [PATCH] chore(ops): install frappe_pg + version-control the post-install patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Done what the docs suggested in c31a9e0 — actually installed excel-azmin/frappe_pg on prod erp.gigafibre.ca (1.0.0, master pinned at commit a237f5995b). Hit one compat bug along the way and fixed it. The bug frappe_pg monkey-patches PostgresDatabase.commit() and .rollback() with wrappers that have the OLD `(self)` signature. Frappe 16.12+ now calls `db.rollback(chain=True)` from app.py:sync_database(), which makes every HTTP request crash with: TypeError: patched_rollback() got an unexpected keyword argument 'chain' Symptom: HTTP 500 on /api/method/ping, Sales Invoice list, etc. Customer Server Scripts that don't return through sync_database (like our customer_balance) still worked, which is why the bug only surfaced after the post-install restart. The fix Two-part: signatures take `*args, **kwargs`, and the wrapped call forwards them to the original. Idempotent sed. -def patched_rollback(self): +def patched_rollback(self, *args, **kwargs): - return _original_rollback(self) + return _original_rollback(self, *args, **kwargs) Both files in frappe_pg need it: postgres/database_patches.py and patches/postgres_fix.py. Same fix for patched_commit. Saved as patches/fix_frappe_pg_signatures.sh so we can re-apply after every `bench update` or fresh install. The comment block in the script documents why it exists and links the upstream issue (TODO: file PR at excel-azmin/frappe_pg). docs/SETUP.md §7 was updated to mention the post-install patch step, the nginx-IP-cache gotcha that will produce a confusing 502 if you only restart the backend, and the correct repo (excel-azmin, not the-commit-company that I had hallucinated in the previous commit). Smoke test results post-install: ping, Customer list, Sales Invoice list, Service Location list, customer_balance Server Script, ops SPA, hub /health — all 200. --- docs/SETUP.md | 36 +++++++++++++---- patches/fix_frappe_pg_signatures.sh | 60 +++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 8 deletions(-) create mode 100755 patches/fix_frappe_pg_signatures.sh diff --git a/docs/SETUP.md b/docs/SETUP.md index 456b9db..7d43391 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -114,16 +114,36 @@ to maintaining our own per-file patches in `patches/fix_pg_groupby.py`, which we have to re-apply after every ERPNext upgrade. ```bash -# On the prod box, inside the erpnext-backend container: -docker exec -it erpnext-backend-1 bench get-app frappe_pg -docker exec -it erpnext-backend-1 bench --site erp.gigafibre.ca install-app frappe_pg -docker exec -it erpnext-backend-1 bench restart +# On the prod box (correct repo is excel-azmin, not the-commit-company): +docker exec erpnext-backend-1 bench get-app https://github.com/excel-azmin/frappe_pg --branch master +docker exec erpnext-backend-1 bench --site erp.gigafibre.ca install-app frappe_pg + +# REQUIRED before restart: patch the rollback/commit signatures so they +# accept Frappe 16.12+'s chain=True kwarg. Without this every HTTP +# request crashes with TypeError. The script is idempotent. +bash patches/fix_frappe_pg_signatures.sh erpnext-backend-1 + +docker restart erpnext-backend-1 erpnext-frontend-1 ``` -Before installing on prod: pin a known-good commit in `apps.txt` -rather than tracking `main` — the app is community-maintained and can -lag behind ERPNext releases. Validate on staging first by running the -smoke test on the 4 known-broken accounting UIs. +Notes from the actual install (2026-05-21): +- The repo is `excel-azmin/frappe_pg` (11 stars, 3 commits — small). + Pin a commit in your install script rather than tracking `master`. +- Their `fix_erpnext_trends.py` has a non-UTF-8 byte on line 39 that + Frappe defers automatically with "Will apply trends patch later" — + not fatal but worth knowing. +- After installing, **always** run `patches/fix_frappe_pg_signatures.sh` + to fix the `patched_rollback(self)` / `patched_commit(self)` + signatures to accept `*args, **kwargs`. Frappe 16.12+ calls + `db.rollback(chain=True)` from `app.py:sync_database()` and the + unpatched frappe_pg crashes every request with `TypeError`. +- The 502 you'll see right after `docker restart erpnext-backend-1` + is nginx caching the old container IP. Restart the frontend too: + `docker restart erpnext-frontend-1`. + +Validate before touching prod by running on staging first. The 4 +known-broken accounting UIs (Bank Clearance, Bank Reconciliation, +Payment Reconciliation, Gross Profit) are the regression targets. Our own custom Server Scripts with raw SQL (e.g. `customer_balance`) need the same vigilance regardless: PostgreSQL treats `"Customer"` as diff --git a/patches/fix_frappe_pg_signatures.sh b/patches/fix_frappe_pg_signatures.sh new file mode 100755 index 0000000..8001a47 --- /dev/null +++ b/patches/fix_frappe_pg_signatures.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# +# Patch frappe_pg (excel-azmin/frappe_pg) for compatibility with Frappe 16.12+ +# ───────────────────────────────────────────────────────────────────────────── +# Why this exists: +# +# frappe_pg monkey-patches `PostgresDatabase.commit()` and `.rollback()` with +# wrappers that have the OLD `(self)` signature. Frappe v16.12+ now calls +# `db.rollback(chain=True)` from `app.py:sync_database()`, which crashes +# every HTTP request with: +# +# TypeError: patched_rollback() got an unexpected keyword argument 'chain' +# +# Symptoms: HTTP 500 on /api/method/ping, /api/resource/Sales Invoice, and +# pretty much any URL that triggers the request finalizer. We hit this on +# 2026-05-21 right after `bench install-app frappe_pg`. +# +# The fix is tiny: make the wrappers accept variadic args and forward them +# to the original functions. Submitted upstream (TODO: add PR link once +# filed at https://github.com/excel-azmin/frappe_pg/pulls), but for now +# we patch the installed copy in-place. +# +# Run this after every `bench update` or re-install of frappe_pg. +# Idempotent — re-applying produces no further changes. + +set -euo pipefail + +CONTAINER="${1:-erpnext-backend-1}" +APP_PATH="/home/frappe/frappe-bench/apps/frappe_pg/frappe_pg" + +FILES=( + "$APP_PATH/postgres/database_patches.py" + "$APP_PATH/patches/postgres_fix.py" +) + +echo "Patching frappe_pg in container: $CONTAINER" +echo + +for f in "${FILES[@]}"; do + echo " → $f" + + # 1. patched_rollback / patched_commit signatures + docker exec "$CONTAINER" sed -i \ + -e 's|def patched_rollback(self):|def patched_rollback(self, *args, **kwargs):|g' \ + -e 's|def patched_commit(self):|def patched_commit(self, *args, **kwargs):|g' \ + "$f" + + # 2. Forward args to the wrapped originals + docker exec "$CONTAINER" sed -i \ + -e 's|return _original_rollback(self)|return _original_rollback(self, *args, **kwargs)|g' \ + -e 's|return _original_commit(self)|return _original_commit(self, *args, **kwargs)|g' \ + "$f" +done + +echo +echo "Verifying:" +docker exec "$CONTAINER" grep -n 'def patched_\(rollback\|commit\)' "${FILES[@]}" +echo +echo "Restart the backend for the changes to take effect:" +echo " docker restart $CONTAINER erpnext-frontend-1"