Skip to content
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

handle ImportError and OSError in extras.pytorch #1141

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

cosmicBboy
Copy link
Contributor

Signed-off-by: Niels Bantilan [email protected]

TL;DR

The introduction of "soft" dependencies in flytekit.extras to support pytorch types leads to an OSError when trying to import torch: #1032. This PR adds OSError as a potential exception raised when installing pytorch.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

For context, see this slack thread: https://unionai.slack.com/archives/C01H0FN1NJX/p1661365911337069

Flytekit has a plugin system, hosted as separate packages in this directory of the flytekit repo. However, since Flyte is a ML- and Data-aware orchestration tool, it would make sense for the core ML frameworks to have first class support without having to install any plugins.

Therefore, we're establishing a new pattern for plugins in flytekit.extras that has "soft" dependencies on packages. For example, if the user has pytorch installed, the type plugins should automatically be registered. If the user doesn't, it should gracefully handle errors that might result from importing torch.

Tracking Issue

NA

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #1141 (c42638b) into master (c4ba45a) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1141      +/-   ##
==========================================
- Coverage   68.35%   68.34%   -0.02%     
==========================================
  Files         287      287              
  Lines       25911    25915       +4     
  Branches     2900     2901       +1     
==========================================
  Hits        17711    17711              
- Misses       7721     7725       +4     
  Partials      479      479              
Impacted Files Coverage Δ
flytekit/extras/pytorch/__init__.py 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@samhita-alla samhita-alla merged commit 8e91b47 into master Aug 25, 2022
@cosmicBboy cosmicBboy deleted the torch-graceful-error branch August 25, 2022 15:22
eapolinario pushed a commit that referenced this pull request Sep 15, 2022
* 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]>
eapolinario added a commit that referenced this pull request Sep 16, 2022
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants