fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider #3046

floOnfido · 2022-08-09T14:45:17Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #3045

adchia · 2022-08-09T15:28:18Z

Thanks for the PR!

Can you sign your commits please?

adchia · 2022-08-09T15:28:23Z

/ok-to-test

adchia · 2022-08-09T15:31:51Z

sdk/python/feast/infra/offline_stores/snowflake.py

@@ -429,7 +429,7 @@ def _to_arrow_internal(self) -> pa.Table:

    def to_snowflake(self, table_name: str, temporary=False) -> None:
        """Save dataset as a new Snowflake table"""
-        if self.on_demand_feature_views is not None:
+        if self.on_demand_feature_views is not []:


think you can just do if self.on_demand_feature_views ?

codecov-commenter · 2022-08-09T15:40:00Z

Codecov Report

Merging #3046 (380b92e) into master (d4c15e7) will increase coverage by 8.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3046      +/-   ##
==========================================
+ Coverage   67.49%   75.73%   +8.24%     
==========================================
  Files         167      202      +35     
  Lines       14621    16776    +2155     
==========================================
+ Hits         9868    12706    +2838     
+ Misses       4753     4070     -683

Flag	Coverage Δ
integrationtests	`67.07% <100.00%> (-0.42%)`	⬇️
unittests	`58.12% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdk/python/feast/infra/offline_stores/snowflake.py	`84.18% <100.00%> (+2.12%)`	⬆️
sdk/python/feast/infra/utils/aws_utils.py	`62.85% <0.00%> (-14.03%)`	⬇️
sdk/python/feast/type_map.py	`61.72% <0.00%> (-1.55%)`	⬇️
setup.py	`0.00% <0.00%> (ø)`
.../python/feast/infra/feature_servers/base_config.py	`100.00% <0.00%> (ø)`
...n/feast/infra/feature_servers/aws_lambda/config.py	`100.00% <0.00%> (ø)`
...fline_store/test_universal_historical_retrieval.py	`100.00% <0.00%> (ø)`
..._stores/contrib/postgres_offline_store/postgres.py	`35.13% <0.00%> (ø)`
...ontrib/postgres_offline_store/tests/data_source.py	`51.02% <0.00%> (ø)`
...s/contrib/trino_offline_store/tests/data_source.py	`17.64% <0.00%> (ø)`
... and 112 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

achals · 2022-08-09T17:10:23Z

sdk/python/feast/infra/offline_stores/snowflake.py

@@ -429,7 +429,7 @@ def _to_arrow_internal(self) -> pa.Table:

    def to_snowflake(self, table_name: str, temporary=False) -> None:
        """Save dataset as a new Snowflake table"""
-        if self.on_demand_feature_views is not None:
+        if self.on_demand_feature_views is not []:


Suggested change

if self.on_demand_feature_views is not []:

if self.on_demand_feature_views:

adchia · 2022-08-09T23:05:34Z

you'll also need to sign your commit btw!

Signed-off-by: floOnfido <[email protected]>

floOnfido · 2022-08-10T11:41:08Z

Should be good now @adchia

adchia · 2022-08-10T11:56:55Z

/lgtm

feast-ci-bot · 2022-08-10T12:42:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, floOnfido

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adchia]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…ne for snowflake provider (feast-dev#3046) check if on_demand_feature_views is empty or None Signed-off-by: floOnfido <[email protected]>

## [0.23.1](v0.23.0...v0.23.1) (2022-08-10) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([6af401f](6af401f)) * Fix Feast Java inconsistency with int64 serialization vs python ([#3031](#3031)) ([f340aeb](f340aeb)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([d584ecd](d584ecd)) * More explicit error messages ([#2708](#2708)) ([ca48963](ca48963)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([9a64e77](9a64e77))

…ne for snowflake provider (#3046) check if on_demand_feature_views is empty or None Signed-off-by: floOnfido <[email protected]>

## [0.22.3](v0.22.2...v0.22.3) (2022-08-10) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([67af727](67af727)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([56d645c](56d645c)) * More explicit error messages ([#2708](#2708)) ([93c1c15](93c1c15)) * Move gcp back to 1.47.0 since grpcio-tools 1.48.0 got yanked from pypi ([#2990](#2990)) ([f7e44da](f7e44da)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([329bc47](329bc47))

…ne for snowflake provider (feast-dev#3046) check if on_demand_feature_views is empty or None Signed-off-by: floOnfido <[email protected]> Signed-off-by: Francisco Javier Arceo <[email protected]>

# [0.24.0](v0.23.0...v0.24.0) (2022-08-25) ### Bug Fixes * Check if on_demand_feature_views is an empty list rather than None for snowflake provider ([#3046](#3046)) ([9b05e65](9b05e65)) * FeatureStore.apply applies BatchFeatureView correctly ([#3098](#3098)) ([41be511](41be511)) * Fix Feast Java inconsistency with int64 serialization vs python ([#3031](#3031)) ([4bba787](4bba787)) * Fix feature service inference logic ([#3089](#3089)) ([4310ed7](4310ed7)) * Fix field mapping logic during feature inference ([#3067](#3067)) ([cdfa761](cdfa761)) * Fix incorrect on demand feature view diffing and improve Java tests ([#3074](#3074)) ([0702310](0702310)) * Fix Java helm charts to work with refactored logic. Fix FTS image ([#3105](#3105)) ([2b493e0](2b493e0)) * Fix on demand feature view output in feast plan + Web UI crash ([#3057](#3057)) ([bfae6ac](bfae6ac)) * Fix release workflow to release 0.24.0 ([#3138](#3138)) ([a69aaae](a69aaae)) * Fix Spark offline store type conversion to arrow ([#3071](#3071)) ([b26566d](b26566d)) * Fixing Web UI, which fails for the SQL registry ([#3028](#3028)) ([64603b6](64603b6)) * Force Snowflake Session to Timezone UTC ([#3083](#3083)) ([9f221e6](9f221e6)) * Make infer dummy entity join key idempotent ([#3115](#3115)) ([1f5b1e0](1f5b1e0)) * More explicit error messages ([#2708](#2708)) ([e4d7afd](e4d7afd)) * Parse inline data sources ([#3036](#3036)) ([c7ba370](c7ba370)) * Prevent overwriting existing file during `persist` ([#3088](#3088)) ([69af21f](69af21f)) * Register BatchFeatureView in feature repos correctly ([#3092](#3092)) ([b8e39ea](b8e39ea)) * Return an empty infra object from sql registry when it doesn't exist ([#3022](#3022)) ([8ba87d1](8ba87d1)) * Teardown tables for Snowflake Materialization testing ([#3106](#3106)) ([0a0c974](0a0c974)) * UI error when saved dataset is present in registry. ([#3124](#3124)) ([83cf753](83cf753)) * Update sql.py ([#3096](#3096)) ([2646a86](2646a86)) * Updated snowflake template ([#3130](#3130)) ([f0594e1](f0594e1)) ### Features * Add authentication option for snowflake connector ([#3039](#3039)) ([74c75f1](74c75f1)) * Add Cassandra/AstraDB online store contribution ([#2873](#2873)) ([feb6cb8](feb6cb8)) * Add Snowflake materialization engine ([#2948](#2948)) ([f3b522b](f3b522b)) * Adding saved dataset capabilities for Postgres ([#3070](#3070)) ([d3253c3](d3253c3)) * Allow passing repo config path via flag ([#3077](#3077)) ([0d2d951](0d2d951)) * Contrib azure provider with synapse/mssql offline store and Azure registry store ([#3072](#3072)) ([9f7e557](9f7e557)) * Custom Docker image for Bytewax batch materialization ([#3099](#3099)) ([cdd1b07](cdd1b07)) * Feast AWS Athena offline store (again) ([#3044](#3044)) ([989ce08](989ce08)) * Implement spark offline store `offline_write_batch` method ([#3076](#3076)) ([5b0cc87](5b0cc87)) * Initial Bytewax materialization engine ([#2974](#2974)) ([55c61f9](55c61f9)) * Refactor feature server helm charts to allow passing feature_store.yaml in environment variables ([#3113](#3113)) ([85ee789](85ee789))

feast-ci-bot added the size/XS label Aug 9, 2022

floOnfido changed the title ~~Update snowflake.py~~ fix: check if on_demand_feature_views is [] rather than None for snowflake provider Aug 9, 2022

floOnfido changed the title ~~fix: check if on_demand_feature_views is [] rather than None for snowflake provider~~ fix: check if on_demand_feature_views is empty list rather than None for snowflake provider Aug 9, 2022

floOnfido changed the title ~~fix: check if on_demand_feature_views is empty list rather than None for snowflake provider~~ fix: check if on_demand_feature_views is an empty list rather than None for snowflake provider Aug 9, 2022

floOnfido changed the title ~~fix: check if on_demand_feature_views is an empty list rather than None for snowflake provider~~ fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider Aug 9, 2022

feast-ci-bot added the ok-to-test label Aug 9, 2022

adchia reviewed Aug 9, 2022

View reviewed changes

achals reviewed Aug 9, 2022

View reviewed changes

check if on_demand_feature_views is empty or None

380b92e

Signed-off-by: floOnfido <[email protected]>

flo-mango3 force-pushed the fix-3045 branch from e0c3b7e to 380b92e Compare August 10, 2022 07:13

feast-ci-bot assigned adchia Aug 10, 2022

feast-ci-bot added the lgtm label Aug 10, 2022

adchia approved these changes Aug 10, 2022

View reviewed changes

feast-ci-bot added the approved label Aug 10, 2022

adchia merged commit 9b05e65 into feast-dev:master Aug 10, 2022

floOnfido deleted the fix-3045 branch August 10, 2022 12:53

adchia pushed a commit that referenced this pull request Aug 10, 2022

fix: Check if on_demand_feature_views is an empty list rather than No…

67af727

…ne for snowflake provider (#3046) check if on_demand_feature_views is empty or None Signed-off-by: floOnfido <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider #3046

fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider #3046

floOnfido commented Aug 9, 2022 •

edited

Loading

adchia commented Aug 9, 2022

adchia commented Aug 9, 2022

adchia Aug 9, 2022

codecov-commenter commented Aug 9, 2022 •

edited

Loading

achals Aug 9, 2022

adchia commented Aug 9, 2022

floOnfido commented Aug 10, 2022

adchia commented Aug 10, 2022

feast-ci-bot commented Aug 10, 2022

	if self.on_demand_feature_views is not []:
	if self.on_demand_feature_views:

fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider #3046

fix: Check if on_demand_feature_views is an empty list rather than None for snowflake provider #3046

Conversation

floOnfido commented Aug 9, 2022 • edited Loading

adchia commented Aug 9, 2022

adchia commented Aug 9, 2022

adchia Aug 9, 2022

Choose a reason for hiding this comment

codecov-commenter commented Aug 9, 2022 • edited Loading

Codecov Report

achals Aug 9, 2022

Choose a reason for hiding this comment

adchia commented Aug 9, 2022

floOnfido commented Aug 10, 2022

adchia commented Aug 10, 2022

feast-ci-bot commented Aug 10, 2022

floOnfido commented Aug 9, 2022 •

edited

Loading

codecov-commenter commented Aug 9, 2022 •

edited

Loading