Pydantic model compatibility #1
Conversation
…rtial work for FeatureService pydantic model, and add unit tests for pydantic models.
…, fix FeatureView to not desroy some of its arguments
… on DataSource and Entity, so use those converters in the FeatureView converter. Also, fix a small SparkSource error which imports FeatureView into the SparkSource DataSource definition, breaking the hierarchy of FeatureService to FeatureView to DataSource and Entity, causing circular dependencies
…antic model, and do some linting
sdk/python/feast/infra/offline_stores/contrib/spark_offline_store/spark_source.py
Show resolved
Hide resolved
| @@ -78,7 +78,7 @@ babel==2.12.1 | |||
| # via sphinx | |||
| backcall==0.2.0 | |||
| # via ipython | |||
| backports-zoneinfo==0.2.1 | |||
| backports-zoneinfo==0.2.1;python_version<"3.9" | |||
There was a problem hiding this comment.
Needed? I'm assuming these requirement files are auto generated.
There was a problem hiding this comment.
This is a necessary change to get the py3.8 version of Feast to build.
I added this comment to the Feast dev guide:
feast-dev#2105 (comment)
backports-zoneinfo is only present in the 3.8 requirements, but this package fails with Python 3.9 or higher. Without this requirement, the makefile may choose to attempt to pull a 3.9 version, and fail to build.
There was a problem hiding this comment.
If you look at the ci-requirements.txt for py 3.9 and 3.10, you will not find that dependency. To install feast, we need to use make install-python-ci-dependencies
There was a problem hiding this comment.
It did not build for me when I removed the dependency. I'm not sure what it's covering though. I tend not to like removing items with being sure why I'm removing them.
There was a problem hiding this comment.
Makefile has a problem in getting your current version..PYTHON variable is not defined. Can you define the following variable in PYTHON:=$(shellpython-c'importsys;print(str(sys.version_info[0])+"."+str(sys.version_info[1]))') and run make install-python-ci-dependencies to try out
There was a problem hiding this comment.
Yes, it works for 3.10, which is what I have, but it didn't work for 3.8 until I fixed the above.
There was a problem hiding this comment.
I'll verify and update it if necessary. Thank you.
| @@ -134,6 +134,13 @@ def __init__( | |||
| self.name = name | |||
| self.entities = [e.name for e in entities] if entities else [DUMMY_ENTITY_NAME] | |||
| self.ttl = ttl | |||
|
|
|||
There was a problem hiding this comment.
Does copy method solves your problem?
There was a problem hiding this comment.
Looked into the code. I see they are not constructing Entity object. But we can actually use name to get Entity object. So no change to Feast code.
Please let me know if you see any issues with it.
There was a problem hiding this comment.
So if you look at line 135, they actually overwrite entities with just the names of entities. We need to preserve the actual entities to convert back and forth. Either way this requires a change to feast code. It seems less destructive to make a new property, hence original_entities.
There was a problem hiding this comment.
I also use original_schema to avoid confusion with the @Property schema defined on line 277.
| @@ -134,6 +134,13 @@ def __init__( | |||
| self.name = name | |||
| self.entities = [e.name for e in entities] if entities else [DUMMY_ENTITY_NAME] | |||
| self.ttl = ttl | |||
|
|
|||
There was a problem hiding this comment.
Looked into the code. I see they are not constructing Entity object. But we can actually use name to get Entity object. So no change to Feast code.
Please let me know if you see any issues with it.
Fixed! |
|
Please use |
[All tests and lint checks are passing.]
What this PR does / why we need it:
This PR introduces Pydantic model conversions for Entity, DataSource, and FeatureView, so that they may be transmitted over the wire using FastAPI. This allows a Feast project to be broken up into a local SDK and a remote registry service.
Only RequestSource and SparkSource are supported for now. We plan to add support for FeatureService models in the near future.