-
Notifications
You must be signed in to change notification settings - Fork 305
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
[MSGPACK IDL] Gate feature by setting ENV #2894
Conversation
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2894 +/- ##
===========================================
- Coverage 75.12% 46.97% -28.16%
===========================================
Files 199 199
Lines 20781 20753 -28
Branches 2671 2672 +1
===========================================
- Hits 15612 9748 -5864
- Misses 4400 10521 +6121
+ Partials 769 484 -285 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
@pingsutw |
* Gate MSGPACK IDL feature by setting ENV Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * Add async to def dict_to_old_generic_idl Signed-off-by: Future-Outlier <[email protected]> * print Signed-off-by: Future-Outlier <[email protected]> * Fix structured dataset bug Signed-off-by: Future-Outlier <[email protected]> * dict update Signed-off-by: Future-Outlier <[email protected]> * remvoe breakpoint Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * remove promise.py Signed-off-by: Future-Outlier <[email protected]> * ad tests Signed-off-by: Future-Outlier <[email protected]> * Add Comments Signed-off-by: Future-Outlier <[email protected]> * add commetns Signed-off-by: Future-Outlier <[email protected]> * apply naming suggestion from Kevin Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> * nit Signed-off-by: Future-Outlier <[email protected]> * use flyte_use_old_dc_format as constant Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]>
* Gate MSGPACK IDL feature by setting ENV Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * Add async to def dict_to_old_generic_idl Signed-off-by: Future-Outlier <[email protected]> * print Signed-off-by: Future-Outlier <[email protected]> * Fix structured dataset bug Signed-off-by: Future-Outlier <[email protected]> * dict update Signed-off-by: Future-Outlier <[email protected]> * remvoe breakpoint Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * remove promise.py Signed-off-by: Future-Outlier <[email protected]> * ad tests Signed-off-by: Future-Outlier <[email protected]> * Add Comments Signed-off-by: Future-Outlier <[email protected]> * add commetns Signed-off-by: Future-Outlier <[email protected]> * apply naming suggestion from Kevin Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> * nit Signed-off-by: Future-Outlier <[email protected]> * use flyte_use_old_dc_format as constant Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> Signed-off-by: Katrina Rogan <[email protected]>
* Gate MSGPACK IDL feature by setting ENV Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * Add async to def dict_to_old_generic_idl Signed-off-by: Future-Outlier <[email protected]> * print Signed-off-by: Future-Outlier <[email protected]> * Fix structured dataset bug Signed-off-by: Future-Outlier <[email protected]> * dict update Signed-off-by: Future-Outlier <[email protected]> * remvoe breakpoint Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * update Signed-off-by: Future-Outlier <[email protected]> * remove promise.py Signed-off-by: Future-Outlier <[email protected]> * ad tests Signed-off-by: Future-Outlier <[email protected]> * Add Comments Signed-off-by: Future-Outlier <[email protected]> * add commetns Signed-off-by: Future-Outlier <[email protected]> * apply naming suggestion from Kevin Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> * nit Signed-off-by: Future-Outlier <[email protected]> * use flyte_use_old_dc_format as constant Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> Signed-off-by: 400Ping <[email protected]>
Is this supposed to be a breaking change? One of our integration test failed because of converting dict to binary instead of protobuf struct. |
Sorry now I read the release note more carefully and it is not the problem of this PR (which basically improves backward compatibility). Since we need python <-> java/scala interoperability, what would you suggest in order to support msgpack on java/scala side? https://github.com/msgpack/msgpack-java is it the right lib to use? Thank you. |
Tracking issue
flyteorg/flyte#5318
Why are the changes needed?
We want to ease the pain of generic IDL -> msgpack IDL when the user upgrades to flytekit 1.14.
What changes were proposed in this pull request?
os.environ["FLYTE_USE_OLD_DC_FORMAT"] = "true"
, then use old behavior to generate protobuf struct in the generic IDL.Binary IDL With MessagePack #2760
Binary IDL With MessagePack #2760
Note: Local execution for Protobuf structs is not as fully supported as local execution for msgpack IDL.
I've created an issue and guide my friend to solve it.
flyteorg/flyte#5959
How was this patch tested?
unit test and remote execution.
Setup process
use single binary.
python: 3.12
flyte: master branch
Screenshots
Check all the applicable boxes
Related PRs
Docs link