From 98ac489d36d2b3d953ddd9e35d2bfdebb909988a Mon Sep 17 00:00:00 2001 From: Miles Adkins Date: Wed, 5 Apr 2023 09:34:00 -0500 Subject: [PATCH] fix: Remove snowflake source warehouse tech debt Signed-off-by: Miles Adkins --- protos/feast/core/DataSource.proto | 5 ++-- .../feast/infra/offline_stores/snowflake.py | 8 +------ .../infra/offline_stores/snowflake_source.py | 23 ++++++++----------- .../universal/data_sources/snowflake.py | 1 - sdk/python/tests/unit/test_data_sources.py | 1 - 5 files changed, 12 insertions(+), 26 deletions(-) diff --git a/protos/feast/core/DataSource.proto b/protos/feast/core/DataSource.proto index 3992d2c247..d129086f45 100644 --- a/protos/feast/core/DataSource.proto +++ b/protos/feast/core/DataSource.proto @@ -197,6 +197,8 @@ message DataSource { // Defines options for DataSource that sources features from a Snowflake Query message SnowflakeOptions { + reserved 5; // Snowflake warehouse name + // Snowflake table name string table = 1; @@ -209,9 +211,6 @@ message DataSource { // Snowflake schema name string database = 4; - - // Snowflake warehouse name - string warehouse = 5; } // Defines options for DataSource that sources features from a spark table/query diff --git a/sdk/python/feast/infra/offline_stores/snowflake.py b/sdk/python/feast/infra/offline_stores/snowflake.py index 1dc18256fa..f55a7999fd 100644 --- a/sdk/python/feast/infra/offline_stores/snowflake.py +++ b/sdk/python/feast/infra/offline_stores/snowflake.py @@ -164,7 +164,7 @@ def pull_latest_from_table_or_query( ) select_timestamps = list( map( - lambda field_name: f"to_varchar({field_name}, 'YYYY-MM-DD\"T\"HH24:MI:SS.FFTZH:TZM') as {field_name}", + lambda field_name: f"TO_VARCHAR({field_name}, 'YYYY-MM-DD\"T\"HH24:MI:SS.FFTZH:TZM') AS {field_name}", timestamp_columns, ) ) @@ -178,9 +178,6 @@ def pull_latest_from_table_or_query( ) inner_field_string = ", ".join(select_fields) - if data_source.snowflake_options.warehouse: - config.offline_store.warehouse = data_source.snowflake_options.warehouse - with GetSnowflakeConnection(config.offline_store) as conn: snowflake_conn = conn @@ -232,9 +229,6 @@ def pull_all_from_table_or_query( + '"' ) - if data_source.snowflake_options.warehouse: - config.offline_store.warehouse = data_source.snowflake_options.warehouse - with GetSnowflakeConnection(config.offline_store) as conn: snowflake_conn = conn diff --git a/sdk/python/feast/infra/offline_stores/snowflake_source.py b/sdk/python/feast/infra/offline_stores/snowflake_source.py index 63533214ea..95bd46f1ec 100644 --- a/sdk/python/feast/infra/offline_stores/snowflake_source.py +++ b/sdk/python/feast/infra/offline_stores/snowflake_source.py @@ -1,3 +1,4 @@ +import warnings from typing import Callable, Dict, Iterable, Optional, Tuple from typeguard import typechecked @@ -45,7 +46,6 @@ def __init__( timestamp_field (optional): Event timestamp field used for point in time joins of feature values. database (optional): Snowflake database where the features are stored. - warehouse (optional): Snowflake warehouse where the database is stored. schema (optional): Snowflake schema in which the table is located. table (optional): Snowflake table where the features are stored. Exactly one of 'table' and 'query' must be specified. @@ -60,6 +60,14 @@ def __init__( owner (optional): The owner of the snowflake source, typically the email of the primary maintainer. """ + + if warehouse: + warnings.warn( + "Specifying a warehouse within a SnowflakeSource is to be deprecated." + "Starting v0.32.0, the warehouse as part of the Snowflake store config will be used.", + RuntimeWarning, + ) + if table is None and query is None: raise ValueError('No "table" or "query" argument provided.') if table and query: @@ -73,7 +81,6 @@ def __init__( schema=_schema, table=table, query=query, - warehouse=warehouse, ) # If no name, use the table as the default name. @@ -109,7 +116,6 @@ def from_proto(data_source: DataSourceProto): database=data_source.snowflake_options.database, schema=data_source.snowflake_options.schema, table=data_source.snowflake_options.table, - warehouse=data_source.snowflake_options.warehouse, created_timestamp_column=data_source.created_timestamp_column, field_mapping=dict(data_source.field_mapping), query=data_source.snowflake_options.query, @@ -134,7 +140,6 @@ def __eq__(self, other): and self.schema == other.schema and self.table == other.table and self.query == other.query - and self.warehouse == other.warehouse ) @property @@ -157,11 +162,6 @@ def query(self): """Returns the snowflake options of this snowflake source.""" return self.snowflake_options.query - @property - def warehouse(self): - """Returns the warehouse of this snowflake source.""" - return self.snowflake_options.warehouse - def to_proto(self) -> DataSourceProto: """ Converts a SnowflakeSource object to its protobuf representation. @@ -335,13 +335,11 @@ def __init__( schema: Optional[str], table: Optional[str], query: Optional[str], - warehouse: Optional[str], ): self.database = database or "" self.schema = schema or "" self.table = table or "" self.query = query or "" - self.warehouse = warehouse or "" @classmethod def from_proto(cls, snowflake_options_proto: DataSourceProto.SnowflakeOptions): @@ -359,7 +357,6 @@ def from_proto(cls, snowflake_options_proto: DataSourceProto.SnowflakeOptions): schema=snowflake_options_proto.schema, table=snowflake_options_proto.table, query=snowflake_options_proto.query, - warehouse=snowflake_options_proto.warehouse, ) return snowflake_options @@ -376,7 +373,6 @@ def to_proto(self) -> DataSourceProto.SnowflakeOptions: schema=self.schema, table=self.table, query=self.query, - warehouse=self.warehouse, ) return snowflake_options_proto @@ -393,7 +389,6 @@ def __init__(self, table_ref: str): schema=None, table=table_ref, query=None, - warehouse=None, ) @staticmethod diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py index 257e46df19..c7e5961a88 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py @@ -66,7 +66,6 @@ def create_data_source( timestamp_field=timestamp_field, created_timestamp_column=created_timestamp_column, field_mapping=field_mapping or {"ts_1": "ts"}, - warehouse=self.offline_store_config.warehouse, ) def create_saved_dataset_destination(self) -> SavedDatasetSnowflakeStorage: diff --git a/sdk/python/tests/unit/test_data_sources.py b/sdk/python/tests/unit/test_data_sources.py index 1e8fb75c3e..30b030feb6 100644 --- a/sdk/python/tests/unit/test_data_sources.py +++ b/sdk/python/tests/unit/test_data_sources.py @@ -118,7 +118,6 @@ def test_proto_conversion(): snowflake_source = SnowflakeSource( name="test_source", database="test_database", - warehouse="test_warehouse", schema="test_schema", table="test_table", timestamp_field="event_timestamp",