From 59c33a8c08bf6bb28a2b11fcf5e4d1e7b24806c8 Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 15:37:07 +0100 Subject: [PATCH 1/6] fix: adding self to a method which is failing linting in file.py integration tests, added self param to a method that was failing linting and ignoring other issues Signed-off-by: Dan Baron --- .../feature_repos/universal/data_sources/file.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py index d8b75aca248..772379a4938 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py @@ -451,7 +451,7 @@ def __init__(self, project_name: str, *args, **kwargs): self.server_port: int = 0 self.proc = None - def xdist_groups() -> list[str]: + def xdist_groups(self) -> list[str]: return ["keycloak"] def setup(self, registry: RegistryConfig): @@ -464,10 +464,10 @@ def setup(self, registry: RegistryConfig): entity_key_serialization_version=2, ) - repo_path = Path(tempfile.mkdtemp()) - with open(repo_path / "feature_store.yaml", "w") as outfile: + repo_base_path = Path(tempfile.mkdtemp()) + with open(repo_base_path / "feature_store.yaml", "w") as outfile: yaml.dump(config.model_dump(by_alias=True), outfile) - repo_path = str(repo_path.resolve()) + repo_path = str(repo_base_path.resolve()) include_auth_config( file_path=f"{repo_path}/feature_store.yaml", auth_config=self.auth_config @@ -486,7 +486,7 @@ def setup(self, registry: RegistryConfig): ] self.proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL - ) + ) # type: ignore _time_out_sec: int = 60 # Wait for server to start From 41b01e1020a4e6ea7ad2aeded784d5f135c89c5f Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 15:37:31 +0100 Subject: [PATCH 2/6] fix: added create_table_disposition check when creating a dataset when get_historical_features is called Signed-off-by: Dan Baron --- sdk/python/feast/infra/offline_stores/bigquery.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/infra/offline_stores/bigquery.py b/sdk/python/feast/infra/offline_stores/bigquery.py index 3ee17174619..ed635ae2145 100644 --- a/sdk/python/feast/infra/offline_stores/bigquery.py +++ b/sdk/python/feast/infra/offline_stores/bigquery.py @@ -242,6 +242,7 @@ def get_historical_features( dataset_project, config.offline_store.dataset, config.offline_store.location, + config.offline_store.table_create_disposition, ) entity_schema = _get_entity_schema( @@ -670,6 +671,7 @@ def _get_table_reference_for_new_entity( dataset_project: str, dataset_name: str, dataset_location: Optional[str], + table_create_disposition: str, ) -> str: """Gets the table_id for the new entity to be uploaded.""" @@ -679,8 +681,13 @@ def _get_table_reference_for_new_entity( try: client.get_dataset(dataset.reference) - except NotFound: + except NotFound as nfe: # Only create the dataset if it does not exist + if table_create_disposition == "CREATE_NEVER": + raise ValueError( + f"Dataset {dataset_project}.{dataset_name} does not exist " + f"and table_create_disposition is set to {table_create_disposition}." + ) from nfe client.create_dataset(dataset, exists_ok=True) table_name = offline_utils.get_temp_entity_table_name() From f8d65faf2b6021c039e3680059b44f2a289cbe9e Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 15:37:49 +0100 Subject: [PATCH 3/6] fix: ignoring some mypy linting errors caused by expanding a dict into kwargs in the repo_configuration integration tests Signed-off-by: Dan Baron --- .../tests/integration/feature_repos/repo_configuration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 73f99fb7c28..c688a848362 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -575,9 +575,9 @@ def construct_test_environment( } if not isinstance(offline_creator, RemoteOfflineOidcAuthStoreDataSourceCreator): - environment = Environment(**environment_params) + environment = Environment(**environment_params) # type: ignore else: - environment = OfflineServerPermissionsEnvironment(**environment_params) + environment = OfflineServerPermissionsEnvironment(**environment_params) # type: ignore return environment From 678f7e7e0b6418dee9b9d678027fc745edc6bfc6 Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 15:40:18 +0100 Subject: [PATCH 4/6] ignoring typing in auth_permissions_util that would be unreasonable to fix due to length of type required and imports Signed-off-by: Dan Baron --- sdk/python/tests/utils/auth_permissions_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index b8ca7355e98..49ddd1b530d 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -49,7 +49,7 @@ def default_store( fs = FeatureStore(repo_path=repo_path) - fs.apply(permissions) + fs.apply(permissions) # type: ignore return fs From cdb8539fdc3d4144b7e0fe91521eba4005196e56 Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 15:41:24 +0100 Subject: [PATCH 5/6] fixing method declaration that has no self parameter Signed-off-by: Dan Baron --- .../integration/feature_repos/universal/data_source_creator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py index aa46160358f..5eb9676f408 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py @@ -61,5 +61,5 @@ def create_logged_features_destination(self) -> LoggingDestination: def teardown(self): raise NotImplementedError - def xdist_groups() -> list[str]: + def xdist_groups(self) -> list[str]: return [] From c2f3a74a49121f4d0e9238a95442b84ffa2e5a4b Mon Sep 17 00:00:00 2001 From: Dan Baron Date: Fri, 18 Oct 2024 16:00:59 +0100 Subject: [PATCH 6/6] made xdist_group methods static Signed-off-by: Dan Baron --- .../integration/feature_repos/universal/data_source_creator.py | 3 ++- .../integration/feature_repos/universal/data_sources/file.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py index 5eb9676f408..513a94ee210 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py @@ -61,5 +61,6 @@ def create_logged_features_destination(self) -> LoggingDestination: def teardown(self): raise NotImplementedError - def xdist_groups(self) -> list[str]: + @staticmethod + def xdist_groups() -> list[str]: return [] diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py index 772379a4938..35325c2737e 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py @@ -451,7 +451,8 @@ def __init__(self, project_name: str, *args, **kwargs): self.server_port: int = 0 self.proc = None - def xdist_groups(self) -> list[str]: + @staticmethod + def xdist_groups() -> list[str]: return ["keycloak"] def setup(self, registry: RegistryConfig):