Extend papermill operator to support remote kernels #34840

akshaychitneni · 2023-10-09T23:39:53Z

This PR adds support to run papermill operator that can connect to kernels managed externally by other systems. This would be useful to run the operator in cloud environments and would also be helpful to run spark or scala notebooks

It extends papermill to support new engine using the entry_points as described here

It adds unittest and also a system test to run in CI environments.

Validated using below steps in breeze environment:

* breeze ci-image build --upgrade-to-newer-dependencies
* breeze start-airflow
* pytest --system papermill tests/system/providers/papermill/example_papermill_remote_verify.py

boring-cyborg · 2023-10-09T23:39:57Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: [email protected]
Slack: https://s.apache.org/airflow-slack

Taragolis

The situation around Papermill is not good, this provider might be suspended in the future (or even soon), see:

cc: @eladkal @potiuk

airflow/providers/papermill/operators/papermill.py

airflow/providers/papermill/operators/__init__.py

eladkal · 2023-10-11T06:48:31Z

The situation around Papermill is not good, this provider might be suspended in the future (or even soon), see:

Yes but in the meantime we can accept PRs

akshaychitneni · 2023-10-15T20:29:26Z

The situation around Papermill is not good, this provider might be suspended in the future (or even soon), see:

Comment Remove Airflow 2.3 compat from PapermillOperator #34675 (review)

Support for Python 3.11 nteract/papermill#700

Update builds for Python 3.11 nteract/papermill#708

cc: @eladkal @potiuk

I am happy to contribute/maintain papermill.

akshaychitneni · 2023-10-20T00:01:17Z

@Taragolis @eladkal Could you please take a look?

Taragolis

Need to fix Static Checks
Move generic classes to appropriate places
Create Connection documentation
Additional unit tests

airflow/providers/papermill/operators/papermill.py

Taragolis · 2023-10-25T08:23:07Z

airflow/providers/papermill/operators/papermill.py


 if TYPE_CHECKING:
    from airflow.utils.context import Context

+REMOTE_KERNEL_ENGINE = "remote_kernel_engine"


Could you explain why it should be constant value?

We are registering a remote kernel engine(custom papermill engine) so the operator can work with remote kernels.

airflow/providers/papermill/provider.yaml

Taragolis · 2023-10-25T08:26:12Z

airflow/providers/papermill/operators/papermill.py

+    def shutdown_kernel(self, now: bool = False, restart: bool = False) -> None:
+        pass


Why there is no implementation here?

The operator is not managing the lifecycle of the kernel but only connecting to a remote kernel if configured via hook

airflow/providers/papermill/hooks/kernel.py

airflow/providers/papermill/operators/papermill.py

akshaychitneni · 2023-11-07T18:19:52Z

@Taragolis Thanks for your comments and suggestions. I have updated by PR to address your feedback. Please take a look.

Taragolis

I think you still need to fix Static Checks, most of them fixed by run pre-commit hooks

In additional I think some fixes might required into the documentation formatting, I've add my assumptions, but better to run
breeze build-docs papermill, have a look what the error happen and try to fix it. Some addition useful link which might help to setup local development environment:

If you have any problem you always could ask help in Slack channel #development-first-pr-support

docs/apache-airflow-providers-papermill/connections/jupyter_kernel.rst

akshaychitneni · 2023-11-07T21:04:34Z

I think you still need to fix Static Checks, most of them fixed by run pre-commit hooks

In additional I think some fixes might required into the documentation formatting, I've add my assumptions, but better to run breeze build-docs papermill, have a look what the error happen and try to fix it. Some addition useful link which might help to setup local development environment:

Contributor's Quick Start

CONTRIBUTING.rst

Breeze

If you have any problem you always could ask help in Slack channel #development-first-pr-support

@Taragolis Updated and verified building docs. Thanks

boring-cyborg · 2023-11-13T11:21:38Z

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

boring-cyborg bot added area:providers area:system-tests provider:papermill labels Oct 9, 2023

akshaychitneni changed the title ~~Add remote kernel support for papermill operator~~ Extend papermill operator to support remote kernels Oct 9, 2023

akshaychitneni force-pushed the remote_kernel branch from ca6a3ce to c105472 Compare October 9, 2023 23:42

Taragolis reviewed Oct 10, 2023

View reviewed changes

airflow/providers/papermill/operators/papermill.py Outdated Show resolved Hide resolved

airflow/providers/papermill/operators/__init__.py Outdated Show resolved Hide resolved

akshaychitneni force-pushed the remote_kernel branch 2 times, most recently from 89b463c to a810cb1 Compare October 15, 2023 20:26

akshaychitneni force-pushed the remote_kernel branch 3 times, most recently from acd40ab to 9594640 Compare October 24, 2023 17:46

eladkal requested a review from Taragolis October 25, 2023 06:17

Taragolis reviewed Oct 25, 2023

View reviewed changes

akshaychitneni force-pushed the remote_kernel branch 3 times, most recently from 05823c1 to 125ddbf Compare November 7, 2023 18:12

Taragolis reviewed Nov 7, 2023

View reviewed changes

docs/apache-airflow-providers-papermill/connections/jupyter_kernel.rst Outdated Show resolved Hide resolved

docs/apache-airflow-providers-papermill/connections/jupyter_kernel.rst Outdated Show resolved Hide resolved

akshaychitneni force-pushed the remote_kernel branch 2 times, most recently from 047ab5b to 14ca569 Compare November 7, 2023 21:03

akshaychitneni requested a review from Taragolis November 8, 2023 14:53

akshaychitneni force-pushed the remote_kernel branch 4 times, most recently from 90f52a6 to dcf69aa Compare November 10, 2023 18:03

Add remote kernel support for papermill operator

6364081

akshaychitneni force-pushed the remote_kernel branch from dcf69aa to 6364081 Compare November 11, 2023 19:51

bolkedebruin self-requested a review November 13, 2023 11:21

bolkedebruin approved these changes Nov 13, 2023

View reviewed changes

bolkedebruin merged commit 18dac61 into apache:main Nov 13, 2023
71 checks passed

ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 20, 2023

ephraimbuddy added this to the Airflow 2.8.0 milestone Nov 20, 2023

eladkal mentioned this pull request Nov 24, 2023

Status of testing Providers that were prepared on November 24, 2023 #35845

Closed

69 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend papermill operator to support remote kernels #34840

Extend papermill operator to support remote kernels #34840

akshaychitneni commented Oct 9, 2023

boring-cyborg bot commented Oct 9, 2023

Taragolis left a comment

eladkal commented Oct 11, 2023

akshaychitneni commented Oct 15, 2023

akshaychitneni commented Oct 20, 2023

Taragolis left a comment

Taragolis Oct 25, 2023

akshaychitneni Nov 7, 2023

Taragolis Oct 25, 2023

akshaychitneni Nov 7, 2023

akshaychitneni commented Nov 7, 2023

Taragolis left a comment

akshaychitneni commented Nov 7, 2023 •

edited

Loading

boring-cyborg bot commented Nov 13, 2023

		def shutdown_kernel(self, now: bool = False, restart: bool = False) -> None:
		pass

Extend papermill operator to support remote kernels #34840

Extend papermill operator to support remote kernels #34840

Conversation

akshaychitneni commented Oct 9, 2023

boring-cyborg bot commented Oct 9, 2023

Taragolis left a comment

Choose a reason for hiding this comment

eladkal commented Oct 11, 2023

akshaychitneni commented Oct 15, 2023

akshaychitneni commented Oct 20, 2023

Taragolis left a comment

Choose a reason for hiding this comment

Taragolis Oct 25, 2023

Choose a reason for hiding this comment

akshaychitneni Nov 7, 2023

Choose a reason for hiding this comment

Taragolis Oct 25, 2023

Choose a reason for hiding this comment

akshaychitneni Nov 7, 2023

Choose a reason for hiding this comment

akshaychitneni commented Nov 7, 2023

Taragolis left a comment

Choose a reason for hiding this comment

akshaychitneni commented Nov 7, 2023 • edited Loading

boring-cyborg bot commented Nov 13, 2023

akshaychitneni commented Nov 7, 2023 •

edited

Loading