Set default format of structured dataset to empty

[pingsutw]

Signed-off-by: Kevin Su [email protected]

Compatibility Note

This PR partly relies on flyteorg/flytepropeller@69158ea which was merged into propeller v1.1.36 and Flyte v1.2. If you are running an older version of propeller, you will experience cache-misses with this PR for cached tasks. Please see the tl;dr for more info, but essentially since the format in the signatures of tasks with dataframes in their inputs or outputs will now be "", if you don't upgrade, propeller will think that the signature has changed and result in a data catalog cache-miss.

TL;DR

flyteorg/flyte#2864

The current default format type is Parquet, and it leads to some issues

1. SD transformer will always convert dataframe to parquet instead of cls.DEFAULT_FORMATS[df_type]

def t1() -> StructuredDataset: # The default format of structured dataset is Parquet here

2. Failed to run BQ task if the cache is enabled because type validation is failing.

@task(cache=True, cache_version="1.0")
def t1() -> StructuredDataset: # The default format of structured dataset is Parquet here
    df = pd.DataFrame({"len": [len(sd.open(pd.DataFrame).all())]})
    return StructuredDataset(df, uri=bq_uri) # The format of structured dataset is "" 

In this PR, we set the default format to ""

def t1() -> StructuredDataset: # The default format of structured dataset is "" here

1. By default, we use handlers[df_type][protocol][""] to encode a dataframe. if handlers[df_type][protocol][""] doesn't exist, then use handlers[df_type][protocol][cls.DEFAULT_FORMATS[df_type]].

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

Tracking Issue

flyteorg/flyte#2864

[codecov]

Codecov Report

Merging #1159 (beec6a7) into master (2ccaed7) will increase coverage by 0.58%.
The diff coverage is 76.78%.

@@            Coverage Diff             @@
##           master    #1159      +/-   ##
==========================================
+ Coverage   68.51%   69.09%   +0.58%     
==========================================
  Files         288      295       +7     
  Lines       26085    26983     +898     
  Branches     2918     2537     -381     
==========================================
+ Hits        17871    18644     +773     
- Misses       7735     7843     +108     
- Partials      479      496      +17     

Impacted Files	Coverage Δ
flytekit/clis/helpers.py	94.59% <ø> (ø)
flytekit/clis/sdk_in_container/init.py	100.00% <ø> (+33.33%)	⬆️
flytekit/configuration/internal.py	16.43% <0.00%> (+0.43%)	⬆️
flytekit/core/context_manager.py	39.61% <0.00%> (ø)
flytekit/core/launch_plan.py	57.89% <0.00%> (ø)
flytekit/core/map_task.py	43.11% <0.00%> (ø)
flytekit/deck/__init__.py	0.00% <ø> (ø)
flytekit/extras/tensorflow/__init__.py	0.00% <0.00%> (ø)
flytekit/models/literals.py	40.28% <0.00%> (ø)
flytekit/tools/fast_registration.py	81.53% <0.00%> (-7.53%)	⬇️
... and 101 more

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

[wild-endeavor]

We are going to hold off on this change for a while, need to get the propeller change out and people migrated first. flyteorg/flytepropeller#483

[wild-endeavor]

I think unf. there's some negative interaction with this PR which removed the setting of the default for a lot of the types. I think it's time to address the flaw introduced in that PR - there we took out the default_for_type switch because it was setting the default for both the format and the storage backend. I think we should add two new args, set_default_format and set_default_storage or something like that.