-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add deck to papermill plugin task #1111
Add deck to papermill plugin task #1111
Conversation
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Signed-off-by: Calvin Leather <[email protected]>
Signed-off-by: Calvin Leather <[email protected]>
Signed-off-by: Calvin Leather <[email protected]>
7815b53
to
964fbc7
Compare
Did a quick force push to my branch to --signoff in a rebase. |
Codecov Report
@@ Coverage Diff @@
## master #1111 +/- ##
=======================================
Coverage 86.97% 86.97%
=======================================
Files 276 276
Lines 25511 25511
Branches 2865 2865
=======================================
Hits 22187 22187
Misses 2846 2846
Partials 478 478 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome!! Thank you @CalvinLeather. LGTM
Congrats on merging your first pull request! 🎉 |
Signed-off-by: Calvin Leather <[email protected]>
* Add deck to papermill plugin task (#1111) Signed-off-by: Calvin Leather <[email protected]> * Run compilation even in local execution for dynamic tasks to early detect errors (#1121) Signed-off-by: Yee Hing Tong <[email protected]> * Set to pyflyte run blob object remote when dealing with remote files (#1128) Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> * Override voidPromise resource (#1127) * override void promise resource Signed-off-by: Kevin Su <[email protected]> * override void promise resource Signed-off-by: Kevin Su <[email protected]> * Fix how ShellTask retrieves the Pod class name (#1132) * Fix how ShellTask retrieves the Pod class name Signed-off-by: Matheus Moreno <[email protected]> * Set Pod class name as a constant Signed-off-by: Matheus Moreno <[email protected]> * Revert last commit Signed-off-by: Matheus Moreno <[email protected]> * Execute automatic linting Signed-off-by: Matheus Moreno <[email protected]> Signed-off-by: Matheus Moreno <[email protected]> * Add restriction for pandas to be >=1.2 for fsspec plugin (#1136) Signed-off-by: Yee Hing Tong <[email protected]> * Use joblib hashing to generate cache key to ensure repeatability (#1126) * cherry pick 97b454b Signed-off-by: Yee Hing Tong <[email protected]> * requirements Signed-off-by: Yee Hing Tong <[email protected]> * Fix usage of save in ProtoJoblibHasher Signed-off-by: Eduardo Apolinario <[email protected]> * Regenerate requirements using python 3.7 Signed-off-by: Eduardo Apolinario <[email protected]> * Add test_stable_cache_key Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> * Allow None protocol to mean all data persistence supported storage options in Structured Dataset (#1134) Signed-off-by: Yee Hing Tong <[email protected]> * handle ImportError and OSError in extras.pytorch (#1141) * handle ImportError and OSError in extras.pytorch Signed-off-by: Niels Bantilan <[email protected]> * isolate exception to torch import Signed-off-by: Niels Bantilan <[email protected]> Signed-off-by: Niels Bantilan <[email protected]> * Register dataframe renderers in structured dataset (#1140) * Register dataframe renderers in structured dataset Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * fix test Signed-off-by: Kevin Su <[email protected]> * more tests Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * pyflyte run imperative workflows (#1131) Signed-off-by: Kevin Su <[email protected]> * Using sidecar handler to run Papermill task (#1143) * remove nb prefix Signed-off-by: Kevin Su <[email protected]> * add tests Signed-off-by: Kevin Su <[email protected]> * Update requirements.in Signed-off-by: Kevin Su <[email protected]> * remove _ Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * Properly raise error in NumpyArrayTransformer (#1146) Signed-off-by: Rahul Mehta <[email protected]> Signed-off-by: Rahul Mehta <[email protected]> * Add assert_type in dataclass transformer (#1149) * Add assert_type in dataclassTransformer Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * more tests Signed-off-by: Kevin Su <[email protected]> * fix lint Signed-off-by: Kevin Su <[email protected]> * Add one more test Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * Pickle in Union Type (#1147) * Pickel in Union type Signed-off-by: Kevin Su <[email protected]> * Pickel in Union type Signed-off-by: Kevin Su <[email protected]> * wip Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * update tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * fix tests Signed-off-by: Kevin Su <[email protected]> * Address comment Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * Bump max docker version to 7.0.0 (#1138) Signed-off-by: Rahul Mehta <[email protected]> Signed-off-by: Rahul Mehta <[email protected]> * Set flytekit<2.0 in plugins (#1152) Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> * Add literal type to union literal (#1144) * Add literal type to union literal Signed-off-by: Kevin Su <[email protected]> * fix test Signed-off-by: Kevin Su <[email protected]> * Add tests Signed-off-by: Kevin Su <[email protected]> * more tests Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * Fix the type of optional[int] in nested dataclass (#1148) * Fix the type of optional[int] in nested dataclass Signed-off-by: Kevin Su <[email protected]> * update tests Signed-off-by: Kevin Su <[email protected]> * update comments Signed-off-by: Kevin Su <[email protected]> * nit Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * Added symlink dereferencing in fast packaging and tests (#1151) * Added symlink dereferencing and tests Signed-off-by: Vanshika Chowdhary <[email protected]> * Added flag to register as well Signed-off-by: Vanshika Chowdhary <[email protected]> * More flag propagation Signed-off-by: Vanshika Chowdhary <[email protected]> Signed-off-by: Vanshika Chowdhary <[email protected]> Co-authored-by: Vanshika Chowdhary <[email protected]> * Strip newline from client secret (#1163) * Strip newline from client secret * Add logging and rework the secret file comparison to work on windows Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> * Fix the type of optional[int] in dataclass (#1135) Signed-off-by: Kevin Su <[email protected]> * fix: plugins/flytekit-papermill/dev-requirements.txt to reduce vulnerabilities (#1154) The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-OAUTHLIB-3021142 - https://snyk.io/vuln/SNYK-PYTHON-PYSPARK-3021131 Signed-off-by: Eduardo Apolinario <[email protected]> * Using sidecar handler to run Papermill task (#1143) * remove nb prefix Signed-off-by: Kevin Su <[email protected]> * add tests Signed-off-by: Kevin Su <[email protected]> * Update requirements.in Signed-off-by: Kevin Su <[email protected]> * remove _ Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Kevin Su <[email protected]> * fix: plugins/flytekit-papermill/dev-requirements.txt to reduce vulnerabilities (#1145) The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-COOKIECUTTER-2414281 * Bump pyspark from 3.2.1 to 3.2.2 in /plugins/flytekit-papermill (#1130) Bumps [pyspark](https://github.com/apache/spark) from 3.2.1 to 3.2.2. - [Release notes](https://github.com/apache/spark/releases) - [Commits](apache/spark@v3.2.1...v3.2.2) --- updated-dependencies: - dependency-name: pyspark dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: plugins/flytekit-papermill/dev-requirements.txt to reduce vulnerabilities (#1154) The following vulnerabilities are fixed by pinning transitive dependencies: - https://snyk.io/vuln/SNYK-PYTHON-OAUTHLIB-3021142 - https://snyk.io/vuln/SNYK-PYTHON-PYSPARK-3021131 Signed-off-by: Calvin Leather <[email protected]> Signed-off-by: Yee Hing Tong <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: Kevin Su <[email protected]> Signed-off-by: Matheus Moreno <[email protected]> Signed-off-by: Niels Bantilan <[email protected]> Signed-off-by: Rahul Mehta <[email protected]> Signed-off-by: Vanshika Chowdhary <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Calvin Leather <[email protected]> Co-authored-by: Yee Hing Tong <[email protected]> Co-authored-by: Kevin Su <[email protected]> Co-authored-by: Matheus Moreno <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: Niels Bantilan <[email protected]> Co-authored-by: Rahul Mehta <[email protected]> Co-authored-by: Vanshika Chowdhary <[email protected]> Co-authored-by: Vanshika Chowdhary <[email protected]> Co-authored-by: Snyk bot <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
TL;DR
This PR modifies the papermill plugin so that, if render_deck=True is passed to init, the rendered output html is inserted as a flyte deck.
Type
Are all requirements met?
Complete description
This PR modifies the papermill plugin so that, if a kwarg render_deck is passed, the flyte deck is added automatically, without need for a follow up task. The render_deck kwarg was used to ensure backwards compatibility, by default, the behaviour of the papermill plugin won't be changed (i.e., this deck support is opt-in).
Note that this won't happen for cached execution, there is an open issue out for fixing flyte decks for cached execution.
I tested this change by installing this branch of flytekit into a container and registering one of our company's test notebook tasks, and running it. It gets a deck as intended:
Not 100% happy with how I tested this plugin. Since the unit tests use local execution, the deck/context functionality I think isn't getting used. Let me know if there is a better way to do test this.
Tracking Issue
n/a (can open an issue if you like, JFDI'd this after discussing with Ketan here)
Follow-up issue
NA