From 673f4b9abe4d3f500c536ce23ce71907ddccbe90 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Mon, 7 Feb 2022 16:39:42 -0500 Subject: [PATCH 1/7] Include infra objects in registry dump and fix the from_proto method for Infra Signed-off-by: Danny Chiao --- sdk/python/feast/infra/infra_object.py | 2 +- sdk/python/feast/registry.py | 4 ++++ sdk/python/feast/type_map.py | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/infra_object.py b/sdk/python/feast/infra/infra_object.py index f21016dea5..8ae5222617 100644 --- a/sdk/python/feast/infra/infra_object.py +++ b/sdk/python/feast/infra/infra_object.py @@ -136,7 +136,7 @@ def from_proto(cls, infra_proto: InfraProto): Returns an Infra object created from a protobuf representation. """ infra = cls() - cls.infra_objects += [ + infra.infra_objects += [ InfraObject.from_infra_object_proto(infra_object_proto) for infra_object_proto in infra_proto.infra_objects ] diff --git a/sdk/python/feast/registry.py b/sdk/python/feast/registry.py index 07c4c59b01..8331149aab 100644 --- a/sdk/python/feast/registry.py +++ b/sdk/python/feast/registry.py @@ -830,6 +830,10 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: registry_dict["savedDatasets"].append( MessageToDict(saved_dataset.to_proto()) ) + + registry_dict["infra"].append( + MessageToDict(self.get_infra(project=project).to_proto()) + ) return registry_dict def _prepare_registry_for_changes(self): diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index 82827bce2a..c8d96bf685 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -141,6 +141,9 @@ def python_type_to_feast_value_type( if type_name in type_map: return type_map[type_name] + if isinstance(value, np.bool_): + return ValueType.BOOL + if isinstance(value, np.ndarray) and str(value.dtype) in type_map: item_type = type_map[str(value.dtype)] return ValueType[item_type.name + "_LIST"] From 661b9d83907287f531169448412a60daf2334eec Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Mon, 7 Feb 2022 16:41:59 -0500 Subject: [PATCH 2/7] revert change Signed-off-by: Danny Chiao --- sdk/python/feast/type_map.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sdk/python/feast/type_map.py b/sdk/python/feast/type_map.py index c8d96bf685..82827bce2a 100644 --- a/sdk/python/feast/type_map.py +++ b/sdk/python/feast/type_map.py @@ -141,9 +141,6 @@ def python_type_to_feast_value_type( if type_name in type_map: return type_map[type_name] - if isinstance(value, np.bool_): - return ValueType.BOOL - if isinstance(value, np.ndarray) and str(value.dtype) in type_map: item_type = type_map[str(value.dtype)] return ValueType[item_type.name + "_LIST"] From cac3a5a4ce0bf23430ecaeaf8638fa8ce14ab528 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Tue, 8 Feb 2022 10:58:28 -0500 Subject: [PATCH 3/7] Fix tests Signed-off-by: Danny Chiao --- sdk/python/tests/integration/registration/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/registration/test_cli.py b/sdk/python/tests/integration/registration/test_cli.py index bba12056ce..c3f4b916ef 100644 --- a/sdk/python/tests/integration/registration/test_cli.py +++ b/sdk/python/tests/integration/registration/test_cli.py @@ -59,7 +59,7 @@ def test_universal_cli(test_repo_config) -> None: # Save only the specs, not the metadata. registry_specs = { - key: [fco["spec"] for fco in value] + key: [fco["spec"] if "spec" in fco else fco for fco in value] for key, value in registry_dict.items() } From 1e780c1cb247bca67ac18ff8d60481ab1cf2766b Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Thu, 10 Feb 2022 12:13:44 -0500 Subject: [PATCH 4/7] Fix cli test Signed-off-by: Danny Chiao --- sdk/python/tests/integration/registration/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/registration/test_cli.py b/sdk/python/tests/integration/registration/test_cli.py index c3f4b916ef..6211a43f0d 100644 --- a/sdk/python/tests/integration/registration/test_cli.py +++ b/sdk/python/tests/integration/registration/test_cli.py @@ -105,7 +105,7 @@ def test_universal_cli(test_repo_config) -> None: registry_dict = fs.registry.to_dict(project=project) assertpy.assert_that(registry_specs).is_equal_to( { - key: [fco["spec"] for fco in value] + key: [fco["spec"] if "spec" in fco else fco for fco in value] for key, value in registry_dict.items() } ) From 4133ae0b100ac20a9fc290b6ab74bce7907eb30d Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Sun, 13 Feb 2022 16:50:02 -0500 Subject: [PATCH 5/7] fix test Signed-off-by: Danny Chiao --- sdk/python/feast/infra/infra_object.py | 11 +++++++ .../feast/infra/online_stores/sqlite.py | 3 +- sdk/python/feast/registry.py | 31 ++++++++++++------- sdk/python/feast/repo_operations.py | 2 +- .../feature_repos/repo_configuration.py | 22 ++++++------- .../online_store/test_universal_online.py | 1 + .../integration/registration/test_cli.py | 9 +++--- 7 files changed, 49 insertions(+), 30 deletions(-) diff --git a/sdk/python/feast/infra/infra_object.py b/sdk/python/feast/infra/infra_object.py index 8ae5222617..91770e64e5 100644 --- a/sdk/python/feast/infra/infra_object.py +++ b/sdk/python/feast/infra/infra_object.py @@ -37,6 +37,14 @@ class InfraObject(ABC): Represents a single infrastructure object (e.g. online store table) managed by Feast. """ + @abstractmethod + def __init__(self, name: str): + self._name = name + + @property + def name(self) -> str: + return self._name + @abstractmethod def to_infra_object_proto(self) -> InfraObjectProto: """Converts an InfraObject to its protobuf representation, wrapped in an InfraObjectProto.""" @@ -47,6 +55,9 @@ def to_proto(self) -> Any: """Converts an InfraObject to its protobuf representation.""" pass + def __lt__(self, other) -> bool: + return self.name < other.name + @staticmethod @abstractmethod def from_infra_object_proto(infra_object_proto: InfraObjectProto) -> Any: diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py index 1e7ecf1024..e65aab4e7b 100644 --- a/sdk/python/feast/infra/online_stores/sqlite.py +++ b/sdk/python/feast/infra/online_stores/sqlite.py @@ -249,12 +249,11 @@ class SqliteTable(InfraObject): """ path: str - name: str conn: sqlite3.Connection def __init__(self, path: str, name: str): + super().__init__(name) self.path = path - self.name = name self.conn = _initialize_conn(path) def to_infra_object_proto(self) -> InfraObjectProto: diff --git a/sdk/python/feast/registry.py b/sdk/python/feast/registry.py index 8331149aab..055ab14d3b 100644 --- a/sdk/python/feast/registry.py +++ b/sdk/python/feast/registry.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import json import logging from collections import defaultdict from datetime import datetime, timedelta @@ -21,7 +22,7 @@ from urllib.parse import urlparse from google.protobuf.internal.containers import RepeatedCompositeFieldContainer -from google.protobuf.json_format import MessageToDict +from google.protobuf.json_format import MessageToDict, MessageToJson from proto import Message from feast.base_feature_view import BaseFeatureView @@ -797,45 +798,53 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]: for entity in sorted( self.list_entities(project=project), key=lambda entity: entity.name ): - registry_dict["entities"].append(MessageToDict(entity.to_proto())) + registry_dict["entities"].append( + self._message_to_sorted_dict(entity.to_proto()) + ) for feature_view in sorted( self.list_feature_views(project=project), key=lambda feature_view: feature_view.name, ): - registry_dict["featureViews"].append(MessageToDict(feature_view.to_proto())) + registry_dict["featureViews"].append( + self._message_to_sorted_dict(feature_view.to_proto()) + ) for feature_service in sorted( self.list_feature_services(project=project), key=lambda feature_service: feature_service.name, ): registry_dict["featureServices"].append( - MessageToDict(feature_service.to_proto()) + self._message_to_sorted_dict(feature_service.to_proto()) ) for on_demand_feature_view in sorted( self.list_on_demand_feature_views(project=project), key=lambda on_demand_feature_view: on_demand_feature_view.name, ): registry_dict["onDemandFeatureViews"].append( - MessageToDict(on_demand_feature_view.to_proto()) + self._message_to_sorted_dict(on_demand_feature_view.to_proto()) ) for request_feature_view in sorted( self.list_request_feature_views(project=project), key=lambda request_feature_view: request_feature_view.name, ): registry_dict["requestFeatureViews"].append( - MessageToDict(request_feature_view.to_proto()) + self._message_to_sorted_dict(request_feature_view.to_proto()) ) for saved_dataset in sorted( self.list_saved_datasets(project=project), key=lambda item: item.name ): registry_dict["savedDatasets"].append( - MessageToDict(saved_dataset.to_proto()) + self._message_to_sorted_dict(saved_dataset.to_proto()) + ) + for infra_object in sorted(self.get_infra(project=project).infra_objects): + registry_dict["infra"].append( + self._message_to_sorted_dict(infra_object.to_proto()) ) - - registry_dict["infra"].append( - MessageToDict(self.get_infra(project=project).to_proto()) - ) return registry_dict + @staticmethod + def _message_to_sorted_dict(message: Message) -> Dict[str, Any]: + return json.loads(MessageToJson(message, sort_keys=True)) + def _prepare_registry_for_changes(self): """Prepares the Registry for changes by refreshing the cache if necessary.""" try: diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index 8a3a202c6d..3b0f528e09 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -270,7 +270,7 @@ def registry_dump(repo_config: RepoConfig, repo_path: Path): "breaking changes in the future. No guarantees are made on this interface." ) click.echo(f"{Style.BRIGHT}{Fore.YELLOW}{warning}{Style.RESET_ALL}") - click.echo(json.dumps(registry_dict, indent=2)) + click.echo(json.dumps(registry_dict, indent=2, sort_keys=True)) def cli_check_repo(repo_path: Path): diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 7de2effc5d..9f6f007ebe 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -75,23 +75,23 @@ online_store=REDIS_CONFIG, ), # AWS configurations - IntegrationTestRepoConfig( - provider="aws", - offline_store_creator=RedshiftDataSourceCreator, - online_store=DYNAMO_CONFIG, - python_feature_server=True, - ), + # IntegrationTestRepoConfig( + # provider="aws", + # offline_store_creator=RedshiftDataSourceCreator, + # online_store=DYNAMO_CONFIG, + # python_feature_server=True, + # ), IntegrationTestRepoConfig( provider="aws", offline_store_creator=RedshiftDataSourceCreator, online_store=REDIS_CONFIG, ), # Snowflake configurations - IntegrationTestRepoConfig( - provider="aws", # no list features, no feature server - offline_store_creator=SnowflakeDataSourceCreator, - online_store=REDIS_CONFIG, - ), + # IntegrationTestRepoConfig( + # provider="aws", # no list features, no feature server + # offline_store_creator=SnowflakeDataSourceCreator, + # online_store=REDIS_CONFIG, + # ), ] ) full_repo_configs_module = os.environ.get(FULL_REPO_CONFIGS_MODULE_ENV_NAME) diff --git a/sdk/python/tests/integration/online_store/test_universal_online.py b/sdk/python/tests/integration/online_store/test_universal_online.py index d81eabec39..7d6296baa5 100644 --- a/sdk/python/tests/integration/online_store/test_universal_online.py +++ b/sdk/python/tests/integration/online_store/test_universal_online.py @@ -139,6 +139,7 @@ def test_write_to_online_store_event_check(local_redis_environment): @pytest.mark.integration +@pytest.mark.universal def test_write_to_online_store(environment, universal_data_sources): fs = environment.feature_store entities, datasets, data_sources = universal_data_sources diff --git a/sdk/python/tests/integration/registration/test_cli.py b/sdk/python/tests/integration/registration/test_cli.py index 6211a43f0d..a2c4a9a8e5 100644 --- a/sdk/python/tests/integration/registration/test_cli.py +++ b/sdk/python/tests/integration/registration/test_cli.py @@ -14,7 +14,7 @@ from tests.integration.feature_repos.integration_test_repo_config import ( IntegrationTestRepoConfig, ) -from tests.integration.feature_repos.repo_configuration import FULL_REPO_CONFIGS +from tests.integration.feature_repos.repo_configuration import Environment from tests.integration.feature_repos.universal.data_source_creator import ( DataSourceCreator, ) @@ -32,8 +32,8 @@ @pytest.mark.integration -@pytest.mark.parametrize("test_repo_config", FULL_REPO_CONFIGS) -def test_universal_cli(test_repo_config) -> None: +@pytest.mark.universal +def test_universal_cli(environment: Environment): project = f"test_universal_cli_{str(uuid.uuid4()).replace('-', '')[:8]}" runner = CliRunner() @@ -41,7 +41,7 @@ def test_universal_cli(test_repo_config) -> None: try: repo_path = Path(repo_dir_name) feature_store_yaml = make_feature_store_yaml( - project, test_repo_config, repo_path + project, environment.test_repo_config, repo_path ) repo_config = repo_path / "feature_store.yaml" @@ -56,7 +56,6 @@ def test_universal_cli(test_repo_config) -> None: # Store registry contents, to be compared later. fs = FeatureStore(repo_path=str(repo_path)) registry_dict = fs.registry.to_dict(project=project) - # Save only the specs, not the metadata. registry_specs = { key: [fco["spec"] if "spec" in fco else fco for fco in value] From c00245103a55046a7dee345d390ed2498130eb45 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Sun, 13 Feb 2022 16:52:31 -0500 Subject: [PATCH 6/7] fix test Signed-off-by: Danny Chiao --- sdk/python/feast/registry.py | 2 +- .../feature_repos/repo_configuration.py | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sdk/python/feast/registry.py b/sdk/python/feast/registry.py index 055ab14d3b..4273493255 100644 --- a/sdk/python/feast/registry.py +++ b/sdk/python/feast/registry.py @@ -22,7 +22,7 @@ from urllib.parse import urlparse from google.protobuf.internal.containers import RepeatedCompositeFieldContainer -from google.protobuf.json_format import MessageToDict, MessageToJson +from google.protobuf.json_format import MessageToJson from proto import Message from feast.base_feature_view import BaseFeatureView diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 9f6f007ebe..7de2effc5d 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -75,23 +75,23 @@ online_store=REDIS_CONFIG, ), # AWS configurations - # IntegrationTestRepoConfig( - # provider="aws", - # offline_store_creator=RedshiftDataSourceCreator, - # online_store=DYNAMO_CONFIG, - # python_feature_server=True, - # ), + IntegrationTestRepoConfig( + provider="aws", + offline_store_creator=RedshiftDataSourceCreator, + online_store=DYNAMO_CONFIG, + python_feature_server=True, + ), IntegrationTestRepoConfig( provider="aws", offline_store_creator=RedshiftDataSourceCreator, online_store=REDIS_CONFIG, ), # Snowflake configurations - # IntegrationTestRepoConfig( - # provider="aws", # no list features, no feature server - # offline_store_creator=SnowflakeDataSourceCreator, - # online_store=REDIS_CONFIG, - # ), + IntegrationTestRepoConfig( + provider="aws", # no list features, no feature server + offline_store_creator=SnowflakeDataSourceCreator, + online_store=REDIS_CONFIG, + ), ] ) full_repo_configs_module = os.environ.get(FULL_REPO_CONFIGS_MODULE_ENV_NAME) From 3feecfa2f0d9cd1978d23c3252b546fdc8f02f86 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Sun, 13 Feb 2022 20:15:01 -0500 Subject: [PATCH 7/7] fix test Signed-off-by: Danny Chiao --- sdk/python/feast/infra/online_stores/datastore.py | 3 +-- sdk/python/feast/infra/online_stores/dynamodb.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/infra/online_stores/datastore.py b/sdk/python/feast/infra/online_stores/datastore.py index 5a8d4b7180..a29a8393e2 100644 --- a/sdk/python/feast/infra/online_stores/datastore.py +++ b/sdk/python/feast/infra/online_stores/datastore.py @@ -336,7 +336,6 @@ class DatastoreTable(InfraObject): """ project: str - name: str project_id: Optional[str] namespace: Optional[str] @@ -347,8 +346,8 @@ def __init__( project_id: Optional[str] = None, namespace: Optional[str] = None, ): + super().__init__(name) self.project = project - self.name = name self.project_id = project_id self.namespace = namespace diff --git a/sdk/python/feast/infra/online_stores/dynamodb.py b/sdk/python/feast/infra/online_stores/dynamodb.py index 46592bf2a3..c9ef8d5d67 100644 --- a/sdk/python/feast/infra/online_stores/dynamodb.py +++ b/sdk/python/feast/infra/online_stores/dynamodb.py @@ -227,11 +227,10 @@ class DynamoDBTable(InfraObject): region: The region of the table. """ - name: str region: str def __init__(self, name: str, region: str): - self.name = name + super().__init__(name) self.region = region def to_infra_object_proto(self) -> InfraObjectProto: