fix(docker): stop legacy PyPDF2 3.x shadowing pypdf, breaking multi-record PDF print - #407
Conversation
…ecord PDF print Odoo's setup.py declares an unpinned PyPDF2, so the editable install pulled legacy PyPDF2 3.0.1 next to the pypdf 5.4.0 that OCB's requirements.txt pins for Python 3.13. odoo.tools.pdf probes PyPDF2 first with no version guard, and its removed 1.x API (reader.numPages) raises DeprecationError in _render_qweb_pdf_prepare_streams whenever more than one record is printed. Install Odoo with --no-deps (its real dependencies are already installed from requirements.txt) and add a regression test in spp_base_common that exercises the legacy split API against the image's selected PDF backend. Fixes OP#1168
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #407 +/- ##
==========================================
+ Coverage 71.49% 72.34% +0.84%
==========================================
Files 243 404 +161
Lines 20785 29008 +8223
==========================================
+ Hits 14860 20985 +6125
- Misses 5925 8023 +2098
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Upstream reports filed: odoo/odoo#281660 (odoo/odoo#281660) and OCA/OCB#1346 (OCA/OCB#1346). If either lands a proper fix (version guard in odoo/tools/pdf or a setup.py pin), the |
emjay0921
left a comment
There was a problem hiding this comment.
LGTM — correct root-cause fix, and self-proving in CI.
Root cause (correctly diagnosed): Odoo's setup.py declares an unpinned PyPDF2, so uv pip install -e pulled legacy PyPDF2 3.0.1 alongside the pypdf 5.4.0 that OCB's requirements.txt pins. odoo.tools.pdf probes PyPDF2 before pypdf with no version guard, so 3.x won backend selection — and its removed 1.x API (numPages/getPage) raises DeprecationError in the multi-record split path only.
Fix: --no-deps on the editable Odoo install — safe because the real runtime deps were already installed from the pinned requirements.txt two layers earlier.
Verified (not just trusted):
- CI builds the image from this exact Dockerfile per-SHA (
file: docker/Dockerfile,load: true,tags: openspp-test:${{ github.sha }}) and runs 20 module suites on it — so green CI proves--no-depsdropped no needed runtime dep. - The new
test_pdf_backend.pyexercises the exact legacy split API (numPages/getPage/addPage) throughodoo.tools.pdf; it fails with the productionDeprecationErroron a PyPDF2-3.x image, so its passing proves the rebuilt image now selects pypdf. Genuine TDD guard, well-placed in the always-installed base module. - Confirmed no bundled addon imports
PyPDF2directly, so removing it breaks nothing.
Non-blocking notes:
- Operational: the real fix is the Dockerfile — a running deployment needs an image rebuild, not just a
spp_base_commonmodule upgrade. The 19.0.2.0.2 bump only carries the test + HISTORY; worth calling out in the release note so nobody upgrades the module alone and expects a fix. - Latent assumption:
--no-depsrelies on OCB'srequirements.txtbeing a complete superset of Odoo's runtime deps; a future Odoo bump adding a dep only insetup.pywould be silently dropped — but CI would catch it immediately (import errors), so it's self-guarding. - Cross-repo: this is the root-cause fix for the same PyPDF2-3.0 multi-record print bug flagged on DSWD #1835 — that side benefits once its image rebuilds on the fixed base.
Summary
uv pip install --no-depsindocker/Dockerfile. Odoo'ssetup.pydeclares an unpinnedPyPDF2, so the editable install pulled legacy PyPDF2 3.0.1 alongside the pypdf 5.4.0 that OCB'srequirements.txtpins for Python 3.13 (all of Odoo's real dependencies are already installed from that requirements file two layers earlier).odoo/tools/pdfprobes the legacyPyPDF2package beforepypdfwith no version guard, so PyPDF2 3.x won the backend selection — and its removed 1.x API (reader.numPages,getPage) raisesPyPDF2.errors.DeprecationErrorinir.actions.report._render_qweb_pdf_prepare_streamswhenever a report is printed for more than one record (single-record printing skips the split path, which is why it still worked).spp_base_common(test_pdf_backend.py) that exercises the exact legacy split API against the image's selected PDF backend; it fails with the production error on any image that regresses.spp_base_commonto 19.0.2.0.2 with areadme/HISTORY.mdfragment.OpenProject
Test plan
PyPDF2.errors.DeprecationError: reader.numPages ... removed in PyPDF2 3.0.0— identical to the OP#1168 screenshot./spp t spp_base_commonon the rebuilt image — 7 passed, 0 failed, 0 errorsPyPDF2installed;odoo.tools.pdfselects the_pypdfbackend (pypdf 5.4.0)_render_qweb_pdf('base.report_irmodulereference', res_ids=[<3 ids>])in the running dev container returns a valid PDF through wkhtmltopdf + the multi-record split pathPyPDF2directly (grepped/mnt/extra-addonsand the OCA addon trees)Notes
setup.py:52unpinnedPyPDF2contradicts its ownrequirements.txtpin ofPyPDF==5.4.0for py3.13, andodoo/tools/pdf/_pypdf2_2.pyaccepts PyPDF2 3.x without a version guard. Upstream reports to follow; this PR fixes our image regardless.