Failed to load json_data to dataclass

[pingsutw]

Signed-off-by: Kevin Su pingsutw@apache.org

TL;DR

Fix errors in feast notebook.
Before

>>> print(feature_store)
>>> print(feature_store.config)

During handling of the above exception, another exception occurred:

AttributeError                            Traceback (most recent call last)

/var/folders/by/42kc70lj1bb77klbhf2fmxx00000gn/T/ipykernel_46553/3923996928.py in <module>
      1 print(feature_store)
----> 2 print(feature_store.config)
      3 

~/opt/anaconda3/envs/flyte/lib/python3.8/site-packages/flytekit/core/type_engine.py in __getattr__(self, key)
    766                 return self.__getitem__(key)
    767             except KeyError:
--> 768                 raise AttributeError(key)
    769 
    770     return dataclass_json(dataclasses.dataclass(Model))

AttributeError: config

After
https://github.com/pingsutw/flytesnacks/blob/interactive-feast/cookbook/case_studies/feature_engineering/feast_integration/Feast_Flyte_Demo.ipynb

The problem is that we didn't set the attribute for model class, so we can't load json_data to dataclass

flytekit/flytekit/core/type_engine.py

Line 284 in 7421be2

dc = cast(DataClassJsonMixin, expected_python_type).from_json(_json_format.MessageToJson(lv.scalar.generic))

For example:

@dataclass_json
@dataclass()
class Bar(object):
    x: float
    y: str

We have to set attributes x and y for the model class when we guess data type. otherwise, TypeEngine will keep failing to translate Literal to python value

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

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

https://github.com/lyft/flyte/issues/

Follow-up issue

NA

[codecov]

Codecov Report

Merging #684 (c0922b3) into master (7421be2) will increase coverage by 0.04%.
The diff coverage is 97.05%.

@@            Coverage Diff             @@
##           master     #684      +/-   ##
==========================================
+ Coverage   85.65%   85.69%   +0.04%     
==========================================
  Files         355      355              
  Lines       29661    29683      +22     
  Branches     2415     2425      +10     
==========================================
+ Hits        25406    25438      +32     
+ Misses       3615     3604      -11     
- Partials      640      641       +1     

Impacted Files	Coverage Δ
flytekit/core/type_engine.py	88.36% <95.55%> (+0.54%)	⬆️
tests/flytekit/unit/core/test_type_engine.py	99.71% <100.00%> (+0.01%)	⬆️
flytekit/remote/remote.py	73.25% <0.00%> (-0.20%)	⬇️
flytekit/core/base_task.py	89.04% <0.00%> (ø)
flytekit/remote/workflow_execution.py	89.18% <0.00%> (+16.46%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7421be2...c0922b3. Read the comment docs.

[wild-endeavor]

cc @eapolinario

This PR doesn't fully deprecate https://github.com/flyteorg/flytekit/pull/675/files right? Should we merge that one in first?

For this PR, can we add a unit test that calls to TypeEngine.dict_to_literal_map like this

flytekit/tests/flytekit/unit/core/test_type_engine.py

Line 571 in 7421be2

assert TypeEngine.dict_to_literal_map(ctx, python_value, python_types) == expected_literal_map

[eapolinario]

This PR doesn't fully deprecate https://github.com/flyteorg/flytekit/pull/675/files right? Should we merge that one in first?

Correct, it doesn't, although we could merge this PR first and let #675 be only about handling the inputs+outputs in flyteremote. What do you think, @wild-endeavor ?

[wild-endeavor]

yeah that sounds good. let's still add that dict_to_literal_map unit test to this PR though, then +1