From 53236cdaf8d3b8b769c8eae3f03d3d067bda5bd3 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Mon, 10 Jan 2022 12:16:00 -0500 Subject: [PATCH 1/8] Implement InfraObject.from_proto for easier conversion Signed-off-by: Felix Wang --- sdk/python/feast/errors.py | 5 +++ sdk/python/feast/infra/infra_object.py | 35 +++++++++++++++++-- .../feast/infra/online_stores/datastore.py | 13 +++++++ .../feast/infra/online_stores/dynamodb.py | 6 ++++ .../feast/infra/online_stores/sqlite.py | 4 +++ 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 615069e579..8592960acd 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -293,3 +293,8 @@ def __init__(self, actual_class: str, expected_class: str): super().__init__( f"The registry store class was expected to be {expected_class}, but was instead {actual_class}." ) + + +class FeastInvalidInfraObjectType(Exception): + def __init__(self): + super().__init__("Could not identify the type of the InfraObject.") diff --git a/sdk/python/feast/infra/infra_object.py b/sdk/python/feast/infra/infra_object.py index 282b4bcfab..8d1f794691 100644 --- a/sdk/python/feast/infra/infra_object.py +++ b/sdk/python/feast/infra/infra_object.py @@ -15,9 +15,20 @@ from dataclasses import dataclass, field from typing import Any, List +from feast.errors import FeastInvalidInfraObjectType from feast.importer import import_class +from feast.infra.datastore import DatastoreTable +from feast.infra.dynamodb import DynamoDBTable +from feast.infra.sqlite import SqliteTable +from feast.protos.feast.core.DatastoreTable_pb2 import ( + DatastoreTable as DatastoreTableProto, +) +from feast.protos.feast.core.DynamoDBTable_pb2 import ( + DynamoDBTable as DynamoDBTableProto, +) from feast.protos.feast.core.InfraObject_pb2 import Infra as InfraProto from feast.protos.feast.core.InfraObject_pb2 import InfraObject as InfraObjectProto +from feast.protos.feast.core.SqliteTable_pb2 import SqliteTable as SqliteTableProto DATASTORE_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_stores.datastore.DatastoreTable" DYNAMODB_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_stores.dynamodb.DynamoDBTable" @@ -49,7 +60,7 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: infra_object_proto: A protobuf representation of an InfraObject. Raises: - ValueError: The type of InfraObject could not be identified. + FeastInvalidInfraObjectType: The type of InfraObject could not be identified. """ if infra_object_proto.infra_object_class_type: cls = _get_infra_object_class_from_type( @@ -57,7 +68,27 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: ) return cls.from_infra_object_proto(infra_object_proto) - raise ValueError("Could not identify the type of the InfraObject.") + raise FeastInvalidInfraObjectType() + + @staticmethod + def from_proto(infra_object_proto: Any) -> Any: + """ + Converts a protobuf representation of a subclass to an object of that subclass. + + Args: + infra_object_proto: A protobuf representation of an InfraObject. + + Raises: + FeastInvalidInfraObjectType: The type of InfraObject could not be identified. + """ + if isinstance(infra_object_proto, DatastoreTableProto): + DatastoreTable.from_proto(infra_object_proto) + elif isinstance(infra_object_proto, DynamoDBTableProto): + DynamoDBTable.from_proto(infra_object_proto) + elif isinstance(infra_object_proto, SqliteTableProto): + SqliteTable.from_proto(infra_object_proto) + else: + raise FeastInvalidInfraObjectType() @abstractmethod def update(self): diff --git a/sdk/python/feast/infra/online_stores/datastore.py b/sdk/python/feast/infra/online_stores/datastore.py index 348583a202..6cf2d806dd 100644 --- a/sdk/python/feast/infra/online_stores/datastore.py +++ b/sdk/python/feast/infra/online_stores/datastore.py @@ -387,6 +387,19 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: return datastore_table + @staticmethod + def from_proto(datastore_table_proto: DatastoreTableProto) -> Any: + datastore_table = DatastoreTable( + project=datastore_table_proto.project, name=datastore_table_proto.name, + ) + + if datastore_table_proto.HasField("project_id"): + datastore_table.project_id = datastore_table_proto.project_id.value + if datastore_table_proto.HasField("namespace"): + datastore_table.namespace = datastore_table_proto.namespace.value + + return datastore_table + def update(self): client = _initialize_client(self.project_id, self.namespace) key = client.key("Project", self.project, "Table", self.name) diff --git a/sdk/python/feast/infra/online_stores/dynamodb.py b/sdk/python/feast/infra/online_stores/dynamodb.py index 202cfa54bb..b7f8680e1f 100644 --- a/sdk/python/feast/infra/online_stores/dynamodb.py +++ b/sdk/python/feast/infra/online_stores/dynamodb.py @@ -254,6 +254,12 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: region=infra_object_proto.dynamodb_table.region, ) + @staticmethod + def from_proto(dynamodb_table_proto: DynamoDBTableProto) -> Any: + return DynamoDBTable( + name=dynamodb_table_proto.name, region=dynamodb_table_proto.region, + ) + def update(self): dynamodb_client = _initialize_dynamodb_client(region=self.region) dynamodb_resource = _initialize_dynamodb_resource(region=self.region) diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 2dcbf319c3..c3e1ff0b10 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -261,6 +261,10 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: name=infra_object_proto.sqlite_table.name, ) + @staticmethod + def from_proto(sqlite_table_proto: SqliteTableProto) -> Any: + return SqliteTable(path=sqlite_table_proto.path, name=sqlite_table_proto.name,) + def update(self): self.conn.execute( f"CREATE TABLE IF NOT EXISTS {self.name} (entity_key BLOB, feature_name TEXT, value BLOB, event_ts timestamp, created_ts timestamp, PRIMARY KEY(entity_key, feature_name))" From 828a3dc5fef47efc2d9e77581510ccdd877e2a4c Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Mon, 10 Jan 2022 12:16:05 -0500 Subject: [PATCH 2/8] Implement InfraDiff.update Signed-off-by: Felix Wang --- sdk/python/feast/diff/infra_diff.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/diff/infra_diff.py b/sdk/python/feast/diff/infra_diff.py index d716422261..9dc3db442b 100644 --- a/sdk/python/feast/diff/infra_diff.py +++ b/sdk/python/feast/diff/infra_diff.py @@ -36,7 +36,24 @@ def __init__(self): self.infra_object_diffs = [] def update(self): - pass + """Apply the infrastructure changes specified in this object.""" + for infra_object_diff in self.infra_object_diffs: + if infra_object_diff.transition_type in [ + TransitionType.DELETE, + TransitionType.UPDATE, + ]: + infra_object = InfraObject.from_proto( + infra_object_diff.current_infra_object + ) + infra_object.teardown() + elif infra_object_diff.transition_type in [ + TransitionType.CREATE, + TransitionType.UPDATE, + ]: + infra_object = InfraObject.from_proto( + infra_object_diff.new_infra_object + ) + infra_object.update() def to_string(self): pass From f9421d00f88d0eea9db82a7fa0da590653cb10cf Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Wed, 5 Jan 2022 15:57:04 -0500 Subject: [PATCH 3/8] Modify feature_store.plan to produce an InfraDiff Signed-off-by: Felix Wang --- sdk/python/feast/feature_store.py | 25 ++++++++++++++++--- sdk/python/feast/infra/infra_object.py | 12 ++++----- sdk/python/feast/infra/local.py | 19 ++++++++------ .../feast/infra/online_stores/online_store.py | 14 +++++++++++ .../feast/infra/online_stores/sqlite.py | 16 ++++++++++++ sdk/python/feast/infra/provider.py | 14 +++++++++++ sdk/python/feast/repo_operations.py | 8 +++--- 7 files changed, 87 insertions(+), 21 deletions(-) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index f1fee70336..64bf23ebde 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -38,6 +38,7 @@ from feast import feature_server, flags, flags_helper, utils from feast.base_feature_view import BaseFeatureView from feast.diff.FcoDiff import RegistryDiff +from feast.diff.infra_diff import InfraDiff, diff_infra_protos from feast.entity import Entity from feast.errors import ( EntityNotFoundException, @@ -63,6 +64,7 @@ from feast.infra.provider import Provider, RetrievalJob, get_provider from feast.on_demand_feature_view import OnDemandFeatureView from feast.online_response import OnlineResponse +from feast.protos.feast.core.InfraObject_pb2 import Infra as InfraProto from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.protos.feast.serving.ServingService_pb2 import ( FieldStatus, @@ -405,7 +407,9 @@ def _get_features( return _feature_refs @log_exceptions_and_usage - def plan(self, desired_repo_objects: RepoContents) -> RegistryDiff: + def plan( + self, desired_repo_objects: RepoContents + ) -> Tuple[RegistryDiff, InfraDiff]: """Dry-run registering objects to metadata store. The plan method dry-runs registering one or more definitions (e.g., Entity, FeatureView), and produces @@ -440,7 +444,7 @@ def plan(self, desired_repo_objects: RepoContents) -> RegistryDiff: ... ttl=timedelta(seconds=86400 * 1), ... batch_source=driver_hourly_stats, ... ) - >>> diff = fs.plan(RepoContents({driver_hourly_stats_view}, set(), set(), {driver}, set())) # register entity and feature view + >>> registry_diff, infra_diff = fs.plan(RepoContents({driver_hourly_stats_view}, set(), set(), {driver}, set())) # register entity and feature view """ current_registry_proto = ( @@ -450,8 +454,21 @@ def plan(self, desired_repo_objects: RepoContents) -> RegistryDiff: ) desired_registry_proto = desired_repo_objects.to_registry_proto() - diffs = Registry.diff_between(current_registry_proto, desired_registry_proto) - return diffs + registry_diff = Registry.diff_between( + current_registry_proto, desired_registry_proto + ) + + current_infra_proto = ( + self._registry.cached_registry_proto.infra.__deepcopy__() + if self._registry.cached_registry_proto + else InfraProto() + ) + new_infra_proto = self._provider.plan_infra( + self.config, desired_registry_proto + ).to_proto() + infra_diff = diff_infra_protos(current_infra_proto, new_infra_proto) + + return (registry_diff, infra_diff) @log_exceptions_and_usage def apply( diff --git a/sdk/python/feast/infra/infra_object.py b/sdk/python/feast/infra/infra_object.py index 8d1f794691..41b8e92916 100644 --- a/sdk/python/feast/infra/infra_object.py +++ b/sdk/python/feast/infra/infra_object.py @@ -17,9 +17,6 @@ from feast.errors import FeastInvalidInfraObjectType from feast.importer import import_class -from feast.infra.datastore import DatastoreTable -from feast.infra.dynamodb import DynamoDBTable -from feast.infra.sqlite import SqliteTable from feast.protos.feast.core.DatastoreTable_pb2 import ( DatastoreTable as DatastoreTableProto, ) @@ -82,14 +79,17 @@ def from_proto(infra_object_proto: Any) -> Any: FeastInvalidInfraObjectType: The type of InfraObject could not be identified. """ if isinstance(infra_object_proto, DatastoreTableProto): - DatastoreTable.from_proto(infra_object_proto) + infra_object_class_type = DATASTORE_INFRA_OBJECT_CLASS_TYPE elif isinstance(infra_object_proto, DynamoDBTableProto): - DynamoDBTable.from_proto(infra_object_proto) + infra_object_class_type = DYNAMODB_INFRA_OBJECT_CLASS_TYPE elif isinstance(infra_object_proto, SqliteTableProto): - SqliteTable.from_proto(infra_object_proto) + infra_object_class_type = SQLITE_INFRA_OBJECT_CLASS_TYPE else: raise FeastInvalidInfraObjectType() + cls = _get_infra_object_class_from_type(infra_object_class_type) + return cls.from_proto(infra_object_proto) + @abstractmethod def update(self): """ diff --git a/sdk/python/feast/infra/local.py b/sdk/python/feast/infra/local.py index 31c46cf282..060ac64d53 100644 --- a/sdk/python/feast/infra/local.py +++ b/sdk/python/feast/infra/local.py @@ -1,12 +1,13 @@ import uuid from datetime import datetime from pathlib import Path +from typing import List -from feast.feature_view import FeatureView +from feast.infra.infra_object import Infra, InfraObject from feast.infra.passthrough_provider import PassthroughProvider from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.registry_store import RegistryStore -from feast.repo_config import RegistryConfig +from feast.repo_config import RegistryConfig, RepoConfig from feast.usage import log_exceptions_and_usage @@ -15,11 +16,15 @@ class LocalProvider(PassthroughProvider): This class only exists for backwards compatibility. """ - pass - - -def _table_id(project: str, table: FeatureView) -> str: - return f"{project}_{table.name}" + def plan_infra( + self, config: RepoConfig, desired_registry_proto: RegistryProto + ) -> Infra: + infra_objects: List[InfraObject] = self.online_store.plan( + config, desired_registry_proto + ) + infra = Infra() + infra.infra_objects += infra_objects + return infra class LocalRegistryStore(RegistryStore): diff --git a/sdk/python/feast/infra/online_stores/online_store.py b/sdk/python/feast/infra/online_stores/online_store.py index b2aa1e46d0..1f177996de 100644 --- a/sdk/python/feast/infra/online_stores/online_store.py +++ b/sdk/python/feast/infra/online_stores/online_store.py @@ -18,6 +18,8 @@ from feast import Entity from feast.feature_view import FeatureView +from feast.infra.infra_object import InfraObject +from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto from feast.repo_config import RepoConfig @@ -92,6 +94,18 @@ def update( ): ... + def plan( + self, config: RepoConfig, desired_registry_proto: RegistryProto + ) -> List[InfraObject]: + """ + Returns the set of InfraObjects required to support the desired registry. + + Args: + config: The RepoConfig for the current FeatureStore. + desired_registry_proto: The desired registry, in proto form. + """ + return [] + @abstractmethod def teardown( self, diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index c3e1ff0b10..1e7ecf1024 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -27,6 +27,7 @@ from feast.infra.key_encoding_utils import serialize_entity_key from feast.infra.online_stores.online_store import OnlineStore from feast.protos.feast.core.InfraObject_pb2 import InfraObject as InfraObjectProto +from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.protos.feast.core.SqliteTable_pb2 import SqliteTable as SqliteTableProto from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto @@ -199,6 +200,21 @@ def update( for table in tables_to_delete: conn.execute(f"DROP TABLE IF EXISTS {_table_id(project, table)}") + @log_exceptions_and_usage(online_store="sqlite") + def plan( + self, config: RepoConfig, desired_registry_proto: RegistryProto + ) -> List[InfraObject]: + project = config.project + + infra_objects: List[InfraObject] = [ + SqliteTable( + path=self._get_db_path(config), + name=_table_id(project, FeatureView.from_proto(view)), + ) + for view in desired_registry_proto.feature_views + ] + return infra_objects + def teardown( self, config: RepoConfig, diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py index 3c761f1195..8f9dda9351 100644 --- a/sdk/python/feast/infra/provider.py +++ b/sdk/python/feast/infra/provider.py @@ -12,8 +12,10 @@ from feast.entity import Entity from feast.feature_view import DUMMY_ENTITY_ID, FeatureView from feast.importer import import_class +from feast.infra.infra_object import Infra from feast.infra.offline_stores.offline_store import RetrievalJob from feast.on_demand_feature_view import OnDemandFeatureView +from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto from feast.registry import Registry @@ -61,6 +63,18 @@ def update_infra( """ ... + def plan_infra( + self, config: RepoConfig, desired_registry_proto: RegistryProto + ) -> Infra: + """ + Returns the Infra required to support the desired registry. + + Args: + config: The RepoConfig for the current FeatureStore. + desired_registry_proto: The desired registry, in proto form. + """ + return Infra() + @abc.abstractmethod def teardown_infra( self, project: str, tables: Sequence[FeatureView], entities: Sequence[Entity], diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index 9299a36123..3e9ddb6e30 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -127,20 +127,20 @@ def plan(repo_config: RepoConfig, repo_path: Path, skip_source_validation: bool) for data_source in data_sources: data_source.validate(store.config) - diff = store.plan(repo) + registry_diff, _ = store.plan(repo) views_to_delete = [ v - for v in diff.fco_diffs + for v in registry_diff.fco_diffs if v.fco_type == "feature view" and v.transition_type == TransitionType.DELETE ] views_to_keep = [ v - for v in diff.fco_diffs + for v in registry_diff.fco_diffs if v.fco_type == "feature view" and v.transition_type in {TransitionType.CREATE, TransitionType.UNCHANGED} ] - log_cli_output(diff, views_to_delete, views_to_keep) + log_cli_output(registry_diff, views_to_delete, views_to_keep) def _prepare_registry_and_repo(repo_config, repo_path): From f7accfa1fbfaf120e303726ee257e3cb53e34dd1 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Mon, 10 Jan 2022 18:48:09 -0500 Subject: [PATCH 4/8] Stricter typing for FcoDiff and InfraObjectDiff Signed-off-by: Felix Wang --- sdk/python/feast/diff/FcoDiff.py | 31 ++++++++++++++++++++++------- sdk/python/feast/diff/infra_diff.py | 13 ++++++------ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/sdk/python/feast/diff/FcoDiff.py b/sdk/python/feast/diff/FcoDiff.py index e4b044dcc4..fed20cc874 100644 --- a/sdk/python/feast/diff/FcoDiff.py +++ b/sdk/python/feast/diff/FcoDiff.py @@ -1,20 +1,40 @@ from dataclasses import dataclass -from typing import Any, Iterable, List, Set, Tuple, TypeVar +from typing import Generic, Iterable, List, Set, Tuple, TypeVar from feast.base_feature_view import BaseFeatureView from feast.diff.property_diff import PropertyDiff, TransitionType from feast.entity import Entity from feast.feature_service import FeatureService from feast.protos.feast.core.Entity_pb2 import Entity as EntityProto +from feast.protos.feast.core.FeatureService_pb2 import ( + FeatureService as FeatureServiceProto, +) +from feast.protos.feast.core.FeatureTable_pb2 import FeatureTable as FeatureTableProto from feast.protos.feast.core.FeatureView_pb2 import FeatureView as FeatureViewProto +from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( + OnDemandFeatureView as OnDemandFeatureViewProto, +) +from feast.protos.feast.core.RequestFeatureView_pb2 import ( + RequestFeatureView as RequestFeatureViewProto, +) + +U = TypeVar( + "U", + EntityProto, + FeatureViewProto, + FeatureServiceProto, + FeatureTableProto, + OnDemandFeatureViewProto, + RequestFeatureViewProto, +) @dataclass -class FcoDiff: +class FcoDiff(Generic[U]): name: str fco_type: str - current_fco: Any - new_fco: Any + current_fco: U + new_fco: U fco_property_diffs: List[PropertyDiff] transition_type: TransitionType @@ -46,9 +66,6 @@ def tag_objects_for_keep_delete_add( return objs_to_keep, objs_to_delete, objs_to_add -U = TypeVar("U", EntityProto, FeatureViewProto) - - def tag_proto_objects_for_keep_delete_add( existing_objs: Iterable[U], desired_objs: Iterable[U] ) -> Tuple[Iterable[U], Iterable[U], Iterable[U]]: diff --git a/sdk/python/feast/diff/infra_diff.py b/sdk/python/feast/diff/infra_diff.py index 9dc3db442b..b7d5fca405 100644 --- a/sdk/python/feast/diff/infra_diff.py +++ b/sdk/python/feast/diff/infra_diff.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Any, Iterable, List, Tuple, TypeVar +from typing import Generic, Iterable, List, Tuple, TypeVar from feast.diff.property_diff import PropertyDiff, TransitionType from feast.infra.infra_object import ( @@ -17,13 +17,15 @@ from feast.protos.feast.core.InfraObject_pb2 import Infra as InfraProto from feast.protos.feast.core.SqliteTable_pb2 import SqliteTable as SqliteTableProto +U = TypeVar("U", DatastoreTableProto, DynamoDBTableProto, SqliteTableProto) + @dataclass -class InfraObjectDiff: +class InfraObjectDiff(Generic[U]): name: str infra_object_type: str - current_infra_object: Any - new_infra_object: Any + current_infra_object: U + new_infra_object: U infra_object_property_diffs: List[PropertyDiff] transition_type: TransitionType @@ -59,9 +61,6 @@ def to_string(self): pass -U = TypeVar("U", DatastoreTableProto, DynamoDBTableProto, SqliteTableProto) - - def tag_infra_proto_objects_for_keep_delete_add( existing_objs: Iterable[U], desired_objs: Iterable[U] ) -> Tuple[Iterable[U], Iterable[U], Iterable[U]]: From 168a54659a6eb75177cb3a1719fd54cdebf15b37 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Mon, 10 Jan 2022 19:12:20 -0500 Subject: [PATCH 5/8] Small fixes Signed-off-by: Felix Wang --- sdk/python/feast/infra/infra_object.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/infra/infra_object.py b/sdk/python/feast/infra/infra_object.py index 41b8e92916..f21016dea5 100644 --- a/sdk/python/feast/infra/infra_object.py +++ b/sdk/python/feast/infra/infra_object.py @@ -29,7 +29,7 @@ DATASTORE_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_stores.datastore.DatastoreTable" DYNAMODB_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_stores.dynamodb.DynamoDBTable" -SQLITE_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_store.sqlite.SqliteTable" +SQLITE_INFRA_OBJECT_CLASS_TYPE = "feast.infra.online_stores.sqlite.SqliteTable" class InfraObject(ABC): @@ -125,7 +125,7 @@ def to_proto(self) -> InfraProto: """ infra_proto = InfraProto() for infra_object in self.infra_objects: - infra_object_proto = infra_object.to_proto() + infra_object_proto = infra_object.to_infra_object_proto() infra_proto.infra_objects.append(infra_object_proto) return infra_proto From cd7a97cca466c5a44f7e436eed55b4d53a4e5d67 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Tue, 11 Jan 2022 16:22:30 -0500 Subject: [PATCH 6/8] Fix typevar names Signed-off-by: Felix Wang --- sdk/python/feast/diff/FcoDiff.py | 22 +++++++++++----------- sdk/python/feast/diff/infra_diff.py | 22 ++++++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/sdk/python/feast/diff/FcoDiff.py b/sdk/python/feast/diff/FcoDiff.py index fed20cc874..20b2716269 100644 --- a/sdk/python/feast/diff/FcoDiff.py +++ b/sdk/python/feast/diff/FcoDiff.py @@ -18,8 +18,8 @@ RequestFeatureView as RequestFeatureViewProto, ) -U = TypeVar( - "U", +FcoProto = TypeVar( + "FcoProto", EntityProto, FeatureViewProto, FeatureServiceProto, @@ -30,11 +30,11 @@ @dataclass -class FcoDiff(Generic[U]): +class FcoDiff(Generic[FcoProto]): name: str fco_type: str - current_fco: U - new_fco: U + current_fco: FcoProto + new_fco: FcoProto fco_property_diffs: List[PropertyDiff] transition_type: TransitionType @@ -50,12 +50,12 @@ def add_fco_diff(self, fco_diff: FcoDiff): self.fco_diffs.append(fco_diff) -T = TypeVar("T", Entity, BaseFeatureView, FeatureService) +Fco = TypeVar("Fco", Entity, BaseFeatureView, FeatureService) def tag_objects_for_keep_delete_add( - existing_objs: Iterable[T], desired_objs: Iterable[T] -) -> Tuple[Set[T], Set[T], Set[T]]: + existing_objs: Iterable[Fco], desired_objs: Iterable[Fco] +) -> Tuple[Set[Fco], Set[Fco], Set[Fco]]: existing_obj_names = {e.name for e in existing_objs} desired_obj_names = {e.name for e in desired_objs} @@ -67,8 +67,8 @@ def tag_objects_for_keep_delete_add( def tag_proto_objects_for_keep_delete_add( - existing_objs: Iterable[U], desired_objs: Iterable[U] -) -> Tuple[Iterable[U], Iterable[U], Iterable[U]]: + existing_objs: Iterable[FcoProto], desired_objs: Iterable[FcoProto] +) -> Tuple[Iterable[FcoProto], Iterable[FcoProto], Iterable[FcoProto]]: existing_obj_names = {e.spec.name for e in existing_objs} desired_obj_names = {e.spec.name for e in desired_objs} @@ -82,7 +82,7 @@ def tag_proto_objects_for_keep_delete_add( FIELDS_TO_IGNORE = {"project"} -def diff_between(current: U, new: U, object_type: str) -> FcoDiff: +def diff_between(current: FcoProto, new: FcoProto, object_type: str) -> FcoDiff: assert current.DESCRIPTOR.full_name == new.DESCRIPTOR.full_name property_diffs = [] transition: TransitionType = TransitionType.UNCHANGED diff --git a/sdk/python/feast/diff/infra_diff.py b/sdk/python/feast/diff/infra_diff.py index b7d5fca405..fc79a74f67 100644 --- a/sdk/python/feast/diff/infra_diff.py +++ b/sdk/python/feast/diff/infra_diff.py @@ -17,15 +17,17 @@ from feast.protos.feast.core.InfraObject_pb2 import Infra as InfraProto from feast.protos.feast.core.SqliteTable_pb2 import SqliteTable as SqliteTableProto -U = TypeVar("U", DatastoreTableProto, DynamoDBTableProto, SqliteTableProto) +InfraObjectProto = TypeVar( + "InfraObjectProto", DatastoreTableProto, DynamoDBTableProto, SqliteTableProto +) @dataclass -class InfraObjectDiff(Generic[U]): +class InfraObjectDiff(Generic[InfraObjectProto]): name: str infra_object_type: str - current_infra_object: U - new_infra_object: U + current_infra_object: InfraObjectProto + new_infra_object: InfraObjectProto infra_object_property_diffs: List[PropertyDiff] transition_type: TransitionType @@ -62,8 +64,10 @@ def to_string(self): def tag_infra_proto_objects_for_keep_delete_add( - existing_objs: Iterable[U], desired_objs: Iterable[U] -) -> Tuple[Iterable[U], Iterable[U], Iterable[U]]: + existing_objs: Iterable[InfraObjectProto], desired_objs: Iterable[InfraObjectProto] +) -> Tuple[ + Iterable[InfraObjectProto], Iterable[InfraObjectProto], Iterable[InfraObjectProto] +]: existing_obj_names = {e.name for e in existing_objs} desired_obj_names = {e.name for e in desired_objs} @@ -139,7 +143,7 @@ def diff_infra_protos( def get_infra_object_protos_by_type( infra_proto: InfraProto, infra_object_class_type: str -) -> List[U]: +) -> List[InfraObjectProto]: return [ InfraObject.from_infra_object_proto(infra_object).to_proto() for infra_object in infra_proto.infra_objects @@ -150,7 +154,9 @@ def get_infra_object_protos_by_type( FIELDS_TO_IGNORE = {"project"} -def diff_between(current: U, new: U, infra_object_type: str) -> InfraObjectDiff: +def diff_between( + current: InfraObjectProto, new: InfraObjectProto, infra_object_type: str +) -> InfraObjectDiff: assert current.DESCRIPTOR.full_name == new.DESCRIPTOR.full_name property_diffs = [] transition: TransitionType = TransitionType.UNCHANGED From 6ad515e523bc5ad402efaadf432201f7e5d2d5ee Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Tue, 11 Jan 2022 16:25:32 -0500 Subject: [PATCH 7/8] Add comment Signed-off-by: Felix Wang --- sdk/python/feast/infra/online_stores/datastore.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/python/feast/infra/online_stores/datastore.py b/sdk/python/feast/infra/online_stores/datastore.py index 6cf2d806dd..5a8d4b7180 100644 --- a/sdk/python/feast/infra/online_stores/datastore.py +++ b/sdk/python/feast/infra/online_stores/datastore.py @@ -376,6 +376,7 @@ def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: name=infra_object_proto.datastore_table.name, ) + # Distinguish between null and empty string, since project_id and namespace are StringValues. if infra_object_proto.datastore_table.HasField("project_id"): datastore_table.project_id = ( infra_object_proto.datastore_table.project_id.value @@ -393,6 +394,7 @@ def from_proto(datastore_table_proto: DatastoreTableProto) -> Any: project=datastore_table_proto.project, name=datastore_table_proto.name, ) + # Distinguish between null and empty string, since project_id and namespace are StringValues. if datastore_table_proto.HasField("project_id"): datastore_table.project_id = datastore_table_proto.project_id.value if datastore_table_proto.HasField("namespace"): From 87e417ad4058374ab32e922195863616787c8054 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Tue, 11 Jan 2022 16:25:56 -0500 Subject: [PATCH 8/8] Fix protos Signed-off-by: Felix Wang --- sdk/python/feast/diff/FcoDiff.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/python/feast/diff/FcoDiff.py b/sdk/python/feast/diff/FcoDiff.py index 20b2716269..b85897019f 100644 --- a/sdk/python/feast/diff/FcoDiff.py +++ b/sdk/python/feast/diff/FcoDiff.py @@ -9,7 +9,6 @@ from feast.protos.feast.core.FeatureService_pb2 import ( FeatureService as FeatureServiceProto, ) -from feast.protos.feast.core.FeatureTable_pb2 import FeatureTable as FeatureTableProto from feast.protos.feast.core.FeatureView_pb2 import FeatureView as FeatureViewProto from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( OnDemandFeatureView as OnDemandFeatureViewProto, @@ -23,7 +22,6 @@ EntityProto, FeatureViewProto, FeatureServiceProto, - FeatureTableProto, OnDemandFeatureViewProto, RequestFeatureViewProto, )