Skip to content

[rhoai-2.25] RHAIENG-2345: install Jupyter PDF deps from RPMs#2550

Merged
jiridanek merged 2 commits into
rhoai-2.25from
fix/rhoai-2.25-pdf-deps-rpm
Jul 22, 2026
Merged

[rhoai-2.25] RHAIENG-2345: install Jupyter PDF deps from RPMs#2550
jiridanek merged 2 commits into
rhoai-2.25from
fix/rhoai-2.25-pdf-deps-rpm

Conversation

@jiridanek

@jiridanek jiridanek commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description

Replace flaky Utah/CTAN TexLive + GitHub pandoc fetches in jupyter/utils/install_pdf_deps.sh with dnf RPMs (AppStream texlive + EPEL texlive-tcolorbox / pandoc).

Tracks RHAIENG-2345. Backports the RPM approach already on main (RHAIENG-2186 / ODH #3846), adapted for 2.25 subscribed UBI builds.

Why

Changes

  • Rewrite install_pdf_deps.sh: AppStream texlive-* list + enable EPEL for texlive-tcolorbox and pandoc
  • Drop /usr/local/texlive / /usr/local/pandoc PATH overrides (RPM bins are on /usr/bin)
  • Clear error pointing at #2310 when AppStream packages are missing

Out of scope / notes

Related

  • Main: ODH #2746, #3846
  • RHDS Utah pin bandage: #1982
  • ppc64le helpers install_texlive.sh / install_pandoc.sh left alone (skipped arches / pdf-builder)

How Has This Been Tested?

  • bash -n jupyter/utils/install_pdf_deps.sh
  • Confirmed EPEL9 packages exist: texlive-tcolorbox 20200406-38.el9, pandoc 2.14.0.3-17.el9; epel-release-latest-9 reachable
  • CI: Konflux /build-konflux — minimal cuda/cpu amd64 past install_pdf_deps (no utah.edu)
  • CI: GHA Build Notebooks (subscribed) — datascience/minimal amd64; check-payload no longer fails on static pandoc
  • PDF export / nbconvert smoke on built image
  • Confirm ppc64le/s390x still skip cleanly

Self checklist (all need to be checked):

  • Ensure that you have run make test (gmake on macOS) before asking for review
  • Changes to everything except Dockerfile.konflux files should be done in odh/notebooks and automatically synced to rhds/notebooks. For Konflux-specific changes, modify Dockerfile.konflux files directly in rhds/notebooks as these require special attention in the downstream repository and flow to the upcoming RHOAI release.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Improved PDF export dependency setup across Jupyter images by installing TeX Live and Pandoc through supported system packages.
    • Removed hardcoded tool-path configuration, improving consistency across CPU, CUDA, and ROCm variants.
    • Added installation validation and clearer error handling for unavailable package sources.
    • Preserved architecture-specific behavior for platforms that do not support PDF export dependencies.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

PDF dependency setup

Layer / File(s) Summary
RPM-based PDF dependency installer
jupyter/utils/install_pdf_deps.sh
Replaces archive-based TeX Live and Pandoc installation with EPEL and dnf packages, architecture handling, cleanup, failure guidance, and tool verification.
Dockerfile PDF export wiring and validation
jupyter/*/ubi9-python-3.12/Dockerfile.*, scripts/dockerfile_fragments.py, tests/containers/workbenches/jupyterlab/jupyterlab_test.py
Removes the TeX Live/Pandoc PATH override, retains or adds the installer invocation across image variants, and corrects the notebook filename used by the PDF export test.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: atheo89

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not resolve #2310 because unsubscribed template builds can still fail when AppStream packages are unavailable. Either make the template build work without subscription, or document/skip the workflow when AppStream repos are unavailable.
Out of Scope Changes check ⚠️ Warning The notebook test fixture filename fix in tests/containers/workbenches/jupyterlab/jupyterlab_test.py is unrelated to the PDF dependency change. Move the notebook test typo fix to a separate PR unless it is required for this PDF dependency work.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title is concise and accurately summarizes the main change: installing Jupyter PDF deps from RPMs.
Description check ✅ Passed The description covers the required sections and testing details, though some checklist items and manual-testing confirmation remain unchecked.
📋 Issue Planner

Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).

View plan for ticket: #2310

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rhoai-2.25-pdf-deps-rpm

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
jupyter/utils/install_pdf_deps.sh (1)

64-71: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

Add an image-level PDF export smoke test.

These checks confirm executable and stylesheet discovery, but not complete nbconvert --to pdf functionality. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 93db8d4 and 5f0a9dd.

📒 Files selected for processing (23)
  • jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu
  • jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpu
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cuda
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.rocm
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm
  • jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
  • jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda
  • jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda
  • jupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cuda
  • jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocm
  • jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm
  • jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm
  • jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm
  • jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda
  • jupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda
  • jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu
  • jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu
  • jupyter/utils/install_pdf_deps.sh
  • scripts/check-payload/config.toml
  • scripts/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

@jiridanek

Copy link
Copy Markdown
Member Author

/build-konflux

@jiridanek

Copy link
Copy Markdown
Member Author

@coderabbitai Thanks for the nit — skipping putting nbconvert --to pdf inside install_pdf_deps.sh.

Why: On minimal Dockerfiles that script runs before uv pip install of Jupyter/nbconvert, so an in-installer PDF smoke can't work at that stage. Main's installer also only checks pdflatex / texhash / kpsewhich.

Existing coverage: Image-level smoke is already tests/containers/workbenches/jupyterlab/jupyterlab_test.py::test_pdf_export (jupyter nbconvert … --to pdf, asserts "PDF successfully created") on both main and rhoai-2.25.

Local follow-up (not pushed yet): fixed the longstanding test.ipybntest.ipynb typo in that test (aligned with main's #3472 rename; full cherry-pick of that fixture refactor is out of scope here).

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@jiridanek Thanks for the clear context. Agreed—nbconvert cannot be tested from jupyter/utils/install_pdf_deps.sh because it runs before Jupyter/nbconvert installation, and test_pdf_export is the correct image-level coverage for the full PDF-export path.

The test.ipybntest.ipynb correction sounds like a sensible small follow-up when pushed. I’m withdrawing the nit.

🐇 ✅


✏️ Learnings added
Learnt from: jiridanek
URL: https://github.com/red-hat-data-services/notebooks/pull/2550

Timestamp: 2026-07-22T11:31:05.649Z
Learning: In this repository’s minimal Jupyter Dockerfile builds, `jupyter/utils/install_pdf_deps.sh` runs before `uv pip install` installs Jupyter and nbconvert, so it must not invoke `jupyter nbconvert --to pdf`. PDF export is covered at the image level by `tests/containers/workbenches/jupyterlab/jupyterlab_test.py::test_pdf_export`, which invokes `jupyter nbconvert --to pdf` and asserts `PDF successfully created`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

jiridanek and others added 2 commits July 22, 2026 13:43
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>
@jiridanek
jiridanek force-pushed the fix/rhoai-2.25-pdf-deps-rpm branch from 5f0a9dd to b47916b Compare July 22, 2026 11:43
@jiridanek

Copy link
Copy Markdown
Member Author

/build-konflux

@openshift-ci

openshift-ci Bot commented Jul 22, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ide-developer
Once this PR has been reviewed and has the lgtm label, please assign jiridanek for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jiridanek
jiridanek merged commit e8b1136 into rhoai-2.25 Jul 22, 2026
61 of 103 checks passed
@jiridanek
jiridanek deleted the fix/rhoai-2.25-pdf-deps-rpm branch July 22, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants