-
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
Override Dataclass Serialization/Deserialization Behavior for FlyteTypes
by mashumaro
#2554
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
…_literal function Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
…nd _deserialize methods Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
…eral 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]>
Future-Outlier
requested review from
wild-endeavor,
kumare3,
eapolinario,
pingsutw,
cosmicBboy,
samhita-alla and
thomasjpfan
as code owners
July 3, 2024 18:48
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]>
Future-Outlier
changed the title
Override Serialization/Deserialization Behavior for
Override Serialization/Deserialization Behavior for FlyteTypes
Jul 4, 2024
FlyteFile
and FlyteDirectory
Future-Outlier
commented
Jul 9, 2024
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]>
pingsutw
reviewed
Jul 11, 2024
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: Kevin Su <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
…titem__) for FlyteTypes Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
pingsutw
reviewed
Jul 15, 2024
Signed-off-by: Future-Outlier <[email protected]>
pingsutw
reviewed
Jul 15, 2024
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
pingsutw
approved these changes
Jul 15, 2024
fiedlerNr9
pushed a commit
that referenced
this pull request
Jul 25, 2024
…ypes` by `mashumaro` (#2554) * Add to_json and from_json to Flyte type Signed-off-by: Future-Outlier <[email protected]> * Add to_json and from_json to Flyte type Signed-off-by: Future-Outlier <[email protected]> * remove call of self._serialize_flyte_type in dataclass transformer to_literal function Signed-off-by: Future-Outlier <[email protected]> * uncomment _serialize_flyte_type in dataclass transformer Signed-off-by: Future-Outlier <[email protected]> * use mashumaro SerializableType in flytefile, implemented _serialize and _deserialize methods Signed-off-by: Future-Outlier <[email protected]> * remove flytefile in serialize and deserialize function Signed-off-by: Future-Outlier <[email protected]> * support FlyteDirectory Signed-off-by: Future-Outlier <[email protected]> * uncomment self._serialize_flyte_type() in DataclassTransformer to_literal Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * remove a line Signed-off-by: Future-Outlier <[email protected]> * add print Signed-off-by: Future-Outlier <[email protected]> * uncomment deserialize in dataclass Signed-off-by: Future-Outlier <[email protected]> * remove dataclass json in the dataclass transformer Signed-off-by: Future-Outlier <[email protected]> * remove comments Signed-off-by: Future-Outlier <[email protected]> * remove prints Signed-off-by: Future-Outlier <[email protected]> * update notes Signed-off-by: Future-Outlier <[email protected]> * lint and fix test Signed-off-by: Future-Outlier <[email protected]> * Support structured dataset in dataclass Signed-off-by: Future-Outlier <[email protected]> * add back DataClassJsonMixin inheritance in test Signed-off-by: Future-Outlier <[email protected]> * add flytefile type hints Signed-off-by: Future-Outlier <[email protected]> * Improve type hints and use FlyteContextManager instead of FlyteContext Signed-off-by: Future-Outlier <[email protected]> * rename serialize flyte types to _convert_flyte_type_serializable and deserialize flyte types to _revert_to_flyte_type Signed-off-by: Future-Outlier <[email protected]> * Add comments to describe the dataclass transformer's lifecycle Signed-off-by: Future-Outlier <[email protected]> * rename using _make_flyte_type_serializable Signed-off-by: Future-Outlier <[email protected]> * Add logs to remind users to not use FlyteFile or FlyteDirectory in dataclass Signed-off-by: Future-Outlier <[email protected]> * Add unit tests in test_dataclass.py Signed-off-by: Future-Outlier <[email protected]> * Add Try Catch in dataclass transformer to literal Signed-off-by: Future-Outlier <[email protected]> * support default input Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * generate random prefix for file and dir Signed-off-by: Future-Outlier <[email protected]> * upload successful tests Signed-off-by: Future-Outlier <[email protected]> * update flyteschema behaviour Signed-off-by: Future-Outlier <[email protected]> * add type hints Signed-off-by: Future-Outlier <[email protected]> * fix flyteschema tests Signed-off-by: Future-Outlier <[email protected]> * update tests Signed-off-by: Future-Outlier <[email protected]> * add coverage.xml in .gitignore Signed-off-by: Future-Outlier <[email protected]> * kevin's update Signed-off-by: Kevin Su <[email protected]> * add delattr(cls, "__class_getitem__") Signed-off-by: Future-Outlier <[email protected]> * use AttributeHider to change the behavior of hasattr(cls, __class_getitem__) for FlyteTypes Signed-off-by: Future-Outlier <[email protected]> * remove get_origin() Signed-off-by: Future-Outlier <[email protected]> * add back tests Signed-off-by: Future-Outlier <[email protected]> * Update pingsu's advice Signed-off-by: Future-Outlier <[email protected]> * test: create a variable for DCWithOptional Signed-off-by: Future-Outlier <[email protected]> * remove parent class from the AttributeHider Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Signed-off-by: Kevin Su <[email protected]> Co-authored-by: Kevin Su <[email protected]> Signed-off-by: Jan Fiedler <[email protected]>
This was referenced Jul 25, 2024
[BUG] Mashumaro
JSONEncoder
/JSONDecoder
behavior differs with to_json
/from_json
.
flyteorg/flyte#5588
Closed
mao3267
pushed a commit
to mao3267/flytekit
that referenced
this pull request
Jul 29, 2024
…ypes` by `mashumaro` (flyteorg#2554) * Add to_json and from_json to Flyte type Signed-off-by: Future-Outlier <[email protected]> * Add to_json and from_json to Flyte type Signed-off-by: Future-Outlier <[email protected]> * remove call of self._serialize_flyte_type in dataclass transformer to_literal function Signed-off-by: Future-Outlier <[email protected]> * uncomment _serialize_flyte_type in dataclass transformer Signed-off-by: Future-Outlier <[email protected]> * use mashumaro SerializableType in flytefile, implemented _serialize and _deserialize methods Signed-off-by: Future-Outlier <[email protected]> * remove flytefile in serialize and deserialize function Signed-off-by: Future-Outlier <[email protected]> * support FlyteDirectory Signed-off-by: Future-Outlier <[email protected]> * uncomment self._serialize_flyte_type() in DataclassTransformer to_literal Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * remove a line Signed-off-by: Future-Outlier <[email protected]> * add print Signed-off-by: Future-Outlier <[email protected]> * uncomment deserialize in dataclass Signed-off-by: Future-Outlier <[email protected]> * remove dataclass json in the dataclass transformer Signed-off-by: Future-Outlier <[email protected]> * remove comments Signed-off-by: Future-Outlier <[email protected]> * remove prints Signed-off-by: Future-Outlier <[email protected]> * update notes Signed-off-by: Future-Outlier <[email protected]> * lint and fix test Signed-off-by: Future-Outlier <[email protected]> * Support structured dataset in dataclass Signed-off-by: Future-Outlier <[email protected]> * add back DataClassJsonMixin inheritance in test Signed-off-by: Future-Outlier <[email protected]> * add flytefile type hints Signed-off-by: Future-Outlier <[email protected]> * Improve type hints and use FlyteContextManager instead of FlyteContext Signed-off-by: Future-Outlier <[email protected]> * rename serialize flyte types to _convert_flyte_type_serializable and deserialize flyte types to _revert_to_flyte_type Signed-off-by: Future-Outlier <[email protected]> * Add comments to describe the dataclass transformer's lifecycle Signed-off-by: Future-Outlier <[email protected]> * rename using _make_flyte_type_serializable Signed-off-by: Future-Outlier <[email protected]> * Add logs to remind users to not use FlyteFile or FlyteDirectory in dataclass Signed-off-by: Future-Outlier <[email protected]> * Add unit tests in test_dataclass.py Signed-off-by: Future-Outlier <[email protected]> * Add Try Catch in dataclass transformer to literal Signed-off-by: Future-Outlier <[email protected]> * support default input Signed-off-by: Future-Outlier <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> * generate random prefix for file and dir Signed-off-by: Future-Outlier <[email protected]> * upload successful tests Signed-off-by: Future-Outlier <[email protected]> * update flyteschema behaviour Signed-off-by: Future-Outlier <[email protected]> * add type hints Signed-off-by: Future-Outlier <[email protected]> * fix flyteschema tests Signed-off-by: Future-Outlier <[email protected]> * update tests Signed-off-by: Future-Outlier <[email protected]> * add coverage.xml in .gitignore Signed-off-by: Future-Outlier <[email protected]> * kevin's update Signed-off-by: Kevin Su <[email protected]> * add delattr(cls, "__class_getitem__") Signed-off-by: Future-Outlier <[email protected]> * use AttributeHider to change the behavior of hasattr(cls, __class_getitem__) for FlyteTypes Signed-off-by: Future-Outlier <[email protected]> * remove get_origin() Signed-off-by: Future-Outlier <[email protected]> * add back tests Signed-off-by: Future-Outlier <[email protected]> * Update pingsu's advice Signed-off-by: Future-Outlier <[email protected]> * test: create a variable for DCWithOptional Signed-off-by: Future-Outlier <[email protected]> * remove parent class from the AttributeHider Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Signed-off-by: Kevin Su <[email protected]> Co-authored-by: Kevin Su <[email protected]> Signed-off-by: mao3267 <[email protected]>
3 tasks
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
In the dataclass transformer, we call
_serialize_flyte_types
before converting our dataclass to a JSON string and_deserialize_flyte_type
after converting the JSON string back to dataclasses.This symmetric behavior is implemented to apply certain modifications, allowing Flyte Types to function as desired.
We must move these codes to the
Flyte Types
's method.mashumaro SerializableType
instead ofdataclases_json
's method.Note: pydantic will call
serialize
method from here to serialize values from flyte.Whether this change is introduced or not, it will not affect the behavior of pydantic now.
What changes were proposed in this pull request?
FlyteTypes
.We move code in function serialize flyte types and deserialize flyte types the
_serialize
anddeserialize
function inFlyte Types
.from_json
andto_json
in the dataclass Transformer, since we have to usemashumaro
's to serialize and deserialize, we can't usedataclasess_json
's method.Flyte types: FlyteSchema, FlyteFile, FlyteDirectory and StructuredDataset
Note: I didn't make any changes in
FlyteSchema
because it doesn't do any additional steps in the_serialize_flyte_types
, so there's no reason to implement it.How was this patch tested?
local execution, remote execution and unit tests.
For remote execution, we have to use old image to test backward compatibility.
Setup process
python example
Screenshots
sandbox
demo hosted
https://demo.hosted.unionai.cloud/console/projects/flytesnacks/domains/development/executions/f806fea899824435da55/nodes
dogfood
https://dogfood-gcp.cloud-staging.union.ai/console/projects/flytesnacks/domains/development/executions/f8e1b629a560c441ebdb/nodes
local execution
Check all the applicable boxes
Related PRs
Docs link