Merge feast-snowflake plugin into main repo with documentation by sfc-gh-madkins · Pull Request #2193 · feast-dev/feast

sfc-gh-madkins · 2022-01-05T19:54:11Z

What this PR does / why we need it:

Allows feast to use Snowflake as an Offline & Online Store

Which issue(s) this PR fixes:

Fixes # NONE

Does this PR introduce a user-facing change?:

Allows usage of Snowflake as an offline store.

feast-ci-bot · 2022-01-05T19:54:20Z

Hi @sfc-gh-madkins. Thanks for your PR.

I'm waiting for a feast-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sfc-gh-madkins · 2022-01-05T20:15:54Z

/assign @tsotnet

adchia

you'll also need to change

feast/sdk/python/tests/integration/feature_repos/repo_configuration.py

Line 71 in 94c780b

IntegrationTestRepoConfig(

+ make a data source creator like

feast/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py

Line 25 in 94c780b

self.offline_store_config = RedshiftOfflineStoreConfig(

to include snowflake in the default tests

I setup secrets for this which will be in the environment variables which you can use in tests:

SNOWFLAKE_CI_USER
SNOWFLAKE_CI_PASSWORD
SNOWFLAKE_CI_DEPLOYMENT

docs/getting-started/third-party-integrations.md

docs/reference/data-sources/snowflake.md

docs/tutorials/driver-stats-using-snowflake.md

protos/feast/core/DataSource.proto

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

codecov-commenter · 2022-01-06T15:30:38Z

Codecov Report

Merging #2193 (26270bb) into master (2e4f3e5) will decrease coverage by 0.97%.
The diff coverage is 39.39%.

@@            Coverage Diff             @@
##           master    #2193      +/-   ##
==========================================
- Coverage   59.67%   58.70%   -0.98%     
==========================================
  Files         108      112       +4     
  Lines        9111     9570     +459     
==========================================
+ Hits         5437     5618     +181     
- Misses       3674     3952     +278

Flag	Coverage Δ
unittests	`58.70% <39.39%> (-0.98%)`	⬇️

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

Impacted Files	Coverage Δ
sdk/python/feast/cli.py	`38.65% <ø> (ø)`
sdk/python/feast/data_source.py	`60.15% <0.00%> (-0.73%)`	⬇️
sdk/python/feast/repo_config.py	`77.09% <ø> (ø)`
...fline_store/test_universal_historical_retrieval.py	`23.35% <0.00%> (ø)`
sdk/python/feast/infra/utils/snowflake_utils.py	`23.36% <23.36%> (ø)`
sdk/python/feast/inference.py	`60.60% <25.00%> (-1.90%)`	⬇️
sdk/python/feast/type_map.py	`41.61% <33.33%> (-0.18%)`	⬇️
sdk/python/feast/infra/offline_stores/snowflake.py	`37.93% <37.93%> (ø)`
...hon/feast/infra/offline_stores/snowflake_source.py	`50.42% <50.42%> (ø)`
.../feature_repos/universal/data_sources/snowflake.py	`51.35% <51.35%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4f3e5...26270bb. Read the comment docs.

sfc-gh-madkins · 2022-01-06T17:19:37Z

@adchia Can you also add secrets for SNOWFLAKE_CI_ROLE & SNOWFLAKE_CI_WAREHOUSE and make sure everything is case sensitive -- most objects created in snowflake are UPPERCASE

The account/user should have a database: FEAST and schema: PUBLIC created

adchia · 2022-01-06T20:02:20Z

/ok-to-test

adchia · 2022-01-06T21:54:08Z

@sfc-gh-madkins as mentioned offline, added all the secrets. Let me know when this is ready for another pass (With the datasourcecreator logic all taken cared of)

sdk/python/feast/type_map.py

sdk/python/tests/integration/feature_repos/repo_configuration.py

adchia

final few nits!

sdk/python/requirements/py3.7-ci-requirements.txt

sdk/python/requirements/py3.8-ci-requirements.txt

docs/specs/offline_store_format.md

adchia · 2022-01-18T03:26:21Z

also you need to sign your commits

adchia · 2022-01-18T03:31:26Z

To fix the tests, I think you'll need to pass the secrets to the integration-test-python workflows (i.e. in https://github.com/feast-dev/feast/blob/master/.github/workflows/pr_integration_tests.yml and https://github.com/feast-dev/feast/blob/master/.github/workflows/master_only.yml#L1-L1) with something like how we did the GCP secrets (https://github.com/feast-dev/feast/blob/master/.github/workflows/master_only.yml#L89-L89)

docs/tutorials/driver-stats-using-snowflake.md

pyalex · 2022-01-21T06:03:37Z

Hi @sfc-gh-madkins, please rebase and run make lock-python-ci-dependencies to resolve these conflicts.
Please also fix the DCO. You can probably squash your commits and sign the resulting one.

adchia

bunch of nits

docs/tutorials/driver-stats-using-snowflake.md

sdk/python/feast/data_source.py

sdk/python/feast/infra/utils/snowflake_utils.py

sdk/python/tests/integration/feature_repos/universal/data_sources/snowflake.py

Signed-off-by: david <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

… gone (#2240) * Delete entity from redis when the last attached feature view is deleted Signed-off-by: pyalex <[email protected]> * Delete entity key from Redis only when all attached feature views are gone Signed-off-by: pyalex <[email protected]> * make lint happy Signed-off-by: pyalex <[email protected]> * make lint happy Signed-off-by: pyalex <[email protected]> * one more try with mypy Signed-off-by: pyalex <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Signed-off-by: Michelle Rascati <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Signed-off-by: Judah Rand <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

…ra (#2258) * add snowflake environment vars to test framework Signed-off-by: sfc-gh-madkins <[email protected]> * add snowflake environment vars to test framework Signed-off-by: sfc-gh-madkins <[email protected]>

* Refactor `UNIX_TIMESTAMP` conversion Signed-off-by: Judah Rand <[email protected]> * Return `UNIX_TIMESTAMP` types as `datetime` to user Signed-off-by: Judah Rand <[email protected]> * Fix linting errors Signed-off-by: Judah Rand <[email protected]> * Rename variable to something more sensible Signed-off-by: Judah Rand <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

* Run validation and inference on views and entities during plan Signed-off-by: Felix Wang <[email protected]> * Do not log objects that are unchanged Signed-off-by: Felix Wang <[email protected]> * Rename Fco to FeastObject Signed-off-by: Felix Wang <[email protected]> * Remove useless method Signed-off-by: Felix Wang <[email protected]> * Lint Signed-off-by: Felix Wang <[email protected]> * Always initialize registry during feature store initialization Signed-off-by: Felix Wang <[email protected]> * Fix usage test Signed-off-by: Felix Wang <[email protected]> * Remove print statements Signed-off-by: Felix Wang <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

…y with Java Feast Server (#2259) Signed-off-by: NalinGHub <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Signed-off-by: sfc-gh-madkins <[email protected]>

felixwang9817 · 2022-01-31T07:43:49Z

btw @adchia @sfc-gh-madkins the test currently failing is (I think) unrelated to this PR:

FAILED sdk/python/tests/integration/online_store/test_universal_online.py::test_online_store_cleanup[Provider: aws-Redshif-dynamodb]

I've seen this failure in a bunch of other PRs as well (see e.g. some of the integration tests running on master here; I'm not sure yet if Dynamo is just super flaky or if the test is broken - I just tried running this test locally and it also failed with the same error

the test was introduced by @pyalex in #2240, so maybe he might know what's going on

adchia

/lgtm

feast-ci-bot · 2022-01-31T16:04:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, sfc-gh-madkins

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

The pull request process is described here

Details

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

sfc-gh-madkins requested a review from a team as a code owner January 5, 2022 19:54

sfc-gh-madkins requested review from tsotnet and removed request for a team January 5, 2022 19:54

feast-ci-bot added release-note-none needs-kind labels Jan 5, 2022

feast-ci-bot added needs-ok-to-test size/XXL labels Jan 5, 2022

github-actions bot assigned tsotnet Jan 5, 2022

adchia requested changes Jan 6, 2022

View reviewed changes

feast-ci-bot added release-note and removed release-note-none labels Jan 6, 2022

feast-ci-bot added ok-to-test and removed needs-ok-to-test labels Jan 6, 2022

adchia self-assigned this Jan 6, 2022

sfc-gh-madkins requested a review from adchia January 7, 2022 02:53

adchia reviewed Jan 12, 2022

View reviewed changes

sdk/python/feast/type_map.py Outdated Show resolved Hide resolved

sdk/python/tests/integration/feature_repos/repo_configuration.py Show resolved Hide resolved

sfc-gh-madkins requested a review from adchia January 15, 2022 21:04

adchia approved these changes Jan 18, 2022

View reviewed changes

feast-ci-bot added the approved label Jan 18, 2022

adchia added the kind/feature New feature or request label Jan 18, 2022

feast-ci-bot removed the needs-kind label Jan 18, 2022

adchia reviewed Jan 18, 2022

View reviewed changes

sdk/python/requirements/py3.7-ci-requirements.txt Outdated Show resolved Hide resolved

sdk/python/requirements/py3.8-ci-requirements.txt Outdated Show resolved Hide resolved

docs/specs/offline_store_format.md Outdated Show resolved Hide resolved

damonallison reviewed Jan 18, 2022

View reviewed changes

docs/tutorials/driver-stats-using-snowflake.md Outdated Show resolved Hide resolved

adchia approved these changes Jan 31, 2022

View reviewed changes

dmille and others added 21 commits January 30, 2022 21:37

Add backticks to left_table_query_string (#2250)

f359ee3

Signed-off-by: david <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

historical_field_mappings2 merge for one sign off commit (#2252)

68d2c1a

Signed-off-by: Michelle Rascati <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Correct inconsistent dependency (#2255)

507ca6b

Signed-off-by: Judah Rand <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Squash commits

bb17968

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Add error type and refactor query execution to have retries

1152687

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Handle more snowflake errors

9a2e0a5

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Fix lint errors

7c24117

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Fix lint errors

5f8fbf4

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Fix lint errors

80627c5

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

Fix wrong import

12c92f1

Signed-off-by: Danny Chiao <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

modify registry.db s3 object initialization to work in S3 subdirector…

17874d7

…y with Java Feast Server (#2259) Signed-off-by: NalinGHub <[email protected]> Signed-off-by: sfc-gh-madkins <[email protected]>

clean up docs

61172b3

Signed-off-by: sfc-gh-madkins <[email protected]>

Merge branch 'feast-dev:master' into master

26270bb

lint-python

9e952b2

Signed-off-by: sfc-gh-madkins <[email protected]>

Merge branch 'master' of github.com:sfc-gh-madkins/feast

12b2657

fixed historical test

3ddaf0d

Signed-off-by: sfc-gh-madkins <[email protected]>

fixed historical test

6c29bfb

Signed-off-by: sfc-gh-madkins <[email protected]>

adchia approved these changes Jan 31, 2022

View reviewed changes

feast-ci-bot added the lgtm label Jan 31, 2022

feast-ci-bot merged commit f2bc411 into feast-dev:master Jan 31, 2022

Conversation

sfc-gh-madkins commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feast-ci-bot commented Jan 5, 2022

Uh oh!

sfc-gh-madkins commented Jan 5, 2022

Uh oh!

adchia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jan 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sfc-gh-madkins commented Jan 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adchia commented Jan 6, 2022

Uh oh!

adchia commented Jan 6, 2022

Uh oh!

Uh oh!

Uh oh!

adchia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adchia commented Jan 18, 2022

Uh oh!

adchia commented Jan 18, 2022

Uh oh!

Uh oh!

pyalex commented Jan 21, 2022

Uh oh!

adchia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixwang9817 commented Jan 31, 2022

Uh oh!

adchia left a comment

Choose a reason for hiding this comment

Uh oh!

feast-ci-bot commented Jan 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

sfc-gh-madkins commented Jan 5, 2022 •

edited

Loading

adchia left a comment •

edited

Loading

codecov-commenter commented Jan 6, 2022 •

edited

Loading

sfc-gh-madkins commented Jan 6, 2022 •

edited

Loading