[rhoai-2.25] RHAIENG-2345: install Jupyter PDF deps from RPMs#2550
Conversation
📝 WalkthroughWalkthroughChangesPDF dependency setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
jupyter/utils/install_pdf_deps.sh (1)
64-71: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd an image-level PDF export smoke test.
These checks confirm executable and stylesheet discovery, but not complete
nbconvert --to pdffunctionality. The PR objectives list PDF smoke testing as pending; add it for a subscribed UBI image before relying on this installer across all Dockerfiles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jupyter/utils/install_pdf_deps.sh` around lines 64 - 71, Add an image-level PDF export smoke test to the installer flow after dependency setup, using a minimal notebook and invoking nbconvert with the PDF exporter in the subscribed UBI image. Assert the conversion succeeds and produces a PDF, while retaining the existing executable and stylesheet discovery checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@jupyter/utils/install_pdf_deps.sh`:
- Around line 64-71: Add an image-level PDF export smoke test to the installer
flow after dependency setup, using a minimal notebook and invoking nbconvert
with the PDF exporter in the subscribed UBI image. Assert the conversion
succeeds and produces a PDF, while retaining the existing executable and
stylesheet discovery checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e70b4c4e-8f1b-44ca-ac7b-eb41c17cd23d
📒 Files selected for processing (23)
jupyter/datascience/ubi9-python-3.12/Dockerfile.cpujupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/minimal/ubi9-python-3.12/Dockerfile.rocmjupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudajupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/pytorch/ubi9-python-3.12/Dockerfile.cudajupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/tensorflow/ubi9-python-3.12/Dockerfile.cudajupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/trustyai/ubi9-python-3.12/Dockerfile.cpujupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/utils/install_pdf_deps.shscripts/check-payload/config.tomlscripts/dockerfile_fragments.py
💤 Files with no reviewable changes (21)
- jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu
- jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.rocm
- jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cuda
- jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda
- jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu
- jupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm
- jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda
- scripts/dockerfile_fragments.py
- jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
- jupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm
- jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpu
- jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm
- jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu
- jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocm
- jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm
|
/build-konflux |
|
@coderabbitai Thanks for the nit — skipping putting Why: On minimal Dockerfiles that script runs before Existing coverage: Image-level smoke is already Local follow-up (not pushed yet): fixed the longstanding |
|
Tip For best results, initiate chat on the files or code changes.
The 🐇 ✅ ✏️ Learnings added
|
Replace flaky Utah/CTAN TexLive and GitHub pandoc curls in install_pdf_deps.sh with dnf RPMs (RHAIENG-2345). Aligns 2.25 with main's RPM approach while using EPEL for tcolorbox and pandoc. Co-authored-by: Cursor <cursoragent@cursor.com>
Match main; the longstanding test.ipybn typo was cosmetic for nbconvert but should use the real notebook extension. Co-authored-by: Cursor <cursoragent@cursor.com>
5f0a9dd to
b47916b
Compare
|
/build-konflux |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Description
Replace flaky Utah/CTAN TexLive + GitHub pandoc fetches in
jupyter/utils/install_pdf_deps.shwithdnfRPMs (AppStream texlive + EPELtexlive-tcolorbox/pandoc).Tracks RHAIENG-2345. Backports the RPM approach already on main (RHAIENG-2186 / ODH #3846), adapted for 2.25 subscribed UBI builds.
Why
curltoftp.math.utah.edu(curl: (28)/(60))executable is not dynamically linked)dnf installinstead (RHAIENG-4114)Changes
install_pdf_deps.sh: AppStreamtexlive-*list + enable EPEL fortexlive-tcolorboxandpandoc/usr/local/texlive//usr/local/pandocPATH overrides (RPM bins are on/usr/bin)Out of scope / notes
subscription: false) to still fail; subscribed Konflux/RHDS Build Notebooks should work.dnfflakes — cc @daniellutz.pandoc-rhai/ pylock churn (follow-up: RHAIENG-5765).Related
install_texlive.sh/install_pandoc.shleft alone (skipped arches / pdf-builder)How Has This Been Tested?
bash -n jupyter/utils/install_pdf_deps.shtexlive-tcolorbox20200406-38.el9,pandoc2.14.0.3-17.el9;epel-release-latest-9reachable/build-konflux— minimal cuda/cpu amd64 pastinstall_pdf_deps(no utah.edu)Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Made with Cursor
Summary by CodeRabbit