-
Notifications
You must be signed in to change notification settings - Fork 299
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
Use joblib hashing to generate cache key to ensure repeatability #1126
Conversation
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
nm ignore this for now. need to add more to this. |
@eapolinario can you take a look at this when you get a chance please? I don't understand the unit tests that are failing and can't find any references to explain what's happening. Why do we not want ints to cache? |
Signed-off-by: Eduardo Apolinario <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
+ Coverage 68.24% 68.31% +0.06%
==========================================
Files 287 287
Lines 25808 25873 +65
Branches 2884 2890 +6
==========================================
+ Hits 17612 17674 +62
- Misses 7718 7722 +4
+ Partials 478 477 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
* 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]>
* 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
Thanks to @bethebunny for finding/fixing this.
When the type of a literal is a
dict
orDict
(i.e. no type specified for the value), the type engine converts it to a protobuf struct. If the protobuf struct contains more than one field, then the cache key generated under the old code which ran base64 encoded a cloudpickle dump would sometimes change between calls, which led to local caching not working.Type
Are all requirements met?
Complete description
How did you fix the bug, make the feature etc. Link to any design docs etc
Tracking Issue
https://github.com/flyteorg/flyte/issues/
Follow-up issue
NA
OR
https://github.com/flyteorg/flyte/issues/