SEPURI-SAI-KRISHNA opened a new pull request, #76739:
URL: https://github.com/apache/airflow/pull/76739
`PapermillOperator` accepts `nbconvert` and `nbconvert_args`, carries both
in `template_fields`, and the provider declares a hard `nbconvert>=7.16.1`
dependency. None of it does anything: `execute` runs the notebook and returns,
and nothing in the provider imports or invokes nbconvert.
The conversion was implemented when the parameters were added, in #48222, as
a `jupyter nbconvert` call at the end of `execute`. #48421, which removed auto
lineage, deleted that call and the `import subprocess` a day later while adding
`return self.output_nb` in the same place. Its description does not mention
nbconvert, so this looks like rebase collateral rather than an intended
removal. Both changes are listed in the same release, papermill 3.10.0, the
first under Features and the second under Misc, so the feature has never worked
in a release that advertises it.
This restores the call in the position it previously occupied, before the
return, and documents the two parameters, which were never added to the class
docstring.
I considered two alternatives and did not take either. Driving `nbconvert`
through its Python API instead of the CLI would avoid the subprocess, but
`nbconvert_args` is documented and templated as command line arguments, so
switching would mean redefining or deprecating a public parameter rather than
fixing a bug. Deleting the two parameters and the dependency is defensible,
since nothing can depend on behaviour that never shipped, but it would break
any Dag that already sets `nbconvert=True` and would withdraw a feature the
3.10.0 changelog promised. Happy to go either way if you would prefer one.
The CLI is backed by the declared dependencies rather than assumed to be
present: the provider requires `nbconvert>=7.16.1`, which requires
`jupyter-core>=4.7`, which provides the `jupyter` entry point.
One behaviour change worth flagging: a task that sets `nbconvert=True` today
does nothing and passes, and after this it will run the conversion, so it can
now fail on a notebook or an argument that nbconvert rejects. That is the
documented intent of the parameter, but it is a change for anyone who set it
and never noticed it was inert.
Three deliberate differences from the original block:
The subprocess now runs with `capture_output=True, text=True`. The original
logged `e.stdout` on failure without capturing it, so that message always
printed `None` and the actual nbconvert error was lost.
`nbconvert_args` is rejected with `TypeError` when it is not a list. It is a
templated field, and Jinja renders a templated collection to a string, which
would otherwise be spliced into the command one character per argument.
The HTML path in the final log line is derived with `Path.with_suffix`
instead of `str.replace(".ipynb", ".html")`, which rewrote every occurrence in
the path rather than the extension.
**Tests**
The original change shipped with no tests, which is why its removal went
unnoticed. Added, all in `TestPapermillOperator`:
A parametrized test asserting the exact command, over no extra arguments and
over `["--template", "lab"]` appended after the output notebook.
A test that the default does not convert, so the restored branch stays
behind `nbconvert`.
A test that a templated `nbconvert_args` string raises `TypeError` and does
not run the command, covering the guard above.
All three fail against the operator as it stands, at the patch target rather
than at an assertion: without the restored call there is no `subprocess` import
in the module, so `mock.patch` on it raises `AttributeError: module ... has no
attribute 'subprocess'`.
Also run: the full `test_papermill.py` file (15 passed), and `ruff format`
and `ruff check` on both changed files.
No changelog entry: provider release managers regenerate the changelog from
`git log`, so per-PR entries are not consumed here.
No existing issue; opening the PR directly rather than filing one first.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes - Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
