GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: Add Snowflake materialization engine by sfc-gh-madkins · Pull Request #2948 · feast-dev/feast

Conversation

@sfc-gh-madkins

@sfc-gh-madkins sfc-gh-madkins commented Jul 16, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

Allows Snowflake to be a materialization option when Snowflake is used as an offline store

Which issue(s) this PR fixes:

Fixes #2633

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@feast-ci-bot feast-ci-bot added the size/L label Jul 16, 2022
@sfc-gh-madkins sfc-gh-madkins added do-not-merge/work-in-progress and removed size/L labels Jul 16, 2022
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/materialization/snowflake/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/materialization/snowflake/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/materialization/snowflake/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/materialization/snowflake/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@codecov-commenter

codecov-commenter commented Jul 16, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

Merging #2948 (5d0cf95) into master (83c5efb) will decrease coverage by 9.28%.
The diff coverage is 33.42%.

@@            Coverage Diff             @@
##           master    #2948      +/-   ##
==========================================
- Coverage   67.08%   57.80%   -9.29%     
==========================================
  Files         173      209      +36     
  Lines       15132    17380    +2248     
==========================================
- Hits        10152    10046     -106     
- Misses       4980     7334    +2354
Flag Coverage Δ
integrationtests ?
unittests 57.80% <33.42%> (?)

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

Impacted Files Coverage Δ
...hon/feast/infra/offline_stores/snowflake_source.py 63.04% <14.70%> (-29.27%) ⬇️
...hon/feast/infra/utils/snowflake/snowflake_utils.py 18.96% <20.51%> (ø)
...on/feast/infra/materialization/snowflake_engine.py 34.37% <34.37%> (ø)
...ests/integration/materialization/test_snowflake.py 38.09% <38.09%> (ø)
sdk/python/feast/infra/offline_stores/snowflake.py 37.19% <46.15%> (-47.14%) ⬇️
sdk/python/feast/type_map.py 46.54% <46.66%> (-13.75%) ⬇️
...python/feast/infra/offline_stores/offline_store.py 67.64% <50.00%> (-15.36%) ⬇️
sdk/python/feast/repo_config.py 76.62% <50.00%> (-6.77%) ⬇️
sdk/python/feast/infra/online_stores/snowflake.py 47.66% <71.42%> (-50.51%) ⬇️
.../feature_repos/universal/data_sources/snowflake.py 51.21% <100.00%> (-48.79%) ⬇️
... and 165 more

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/type_map.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/templates/snowflake/driver_repo.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sfc-gh-madkins
sfc-gh-madkins commented Jul 16, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/utils/snowflake_udfs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@feast-ci-bot feast-ci-bot added the size/XL label Jul 17, 2022
sfc-gh-madkins
sfc-gh-madkins commented Jul 21, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/utils/snowflake_udfs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
achals
achals reviewed Jul 22, 2022
View reviewed changes

@achals achals left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks reasonable to me

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread sdk/python/feast/type_map.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/utils/snowflake_udfs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/utils/snowflake_udfs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/utils/snowflake_udfs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
kevjumba
kevjumba reviewed Jul 25, 2022
View reviewed changes

@kevjumba kevjumba left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need some documentation and a few nits.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 2 times, most recently from 905cdb4 to 88984d9 Compare July 30, 2022 13:13
@feast-ci-bot feast-ci-bot added size/XXL and removed size/XL labels Jul 30, 2022
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch from 88984d9 to cf2e3a0 Compare July 30, 2022 14:20
@feast-ci-bot feast-ci-bot added size/XL and removed size/XXL labels Jul 30, 2022
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 2 times, most recently from db2a1b7 to 2c61948 Compare July 31, 2022 02:13
@sfc-gh-madkins sfc-gh-madkins added the ok-to-test label Jul 31, 2022
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 5 times, most recently from 747a85f to fe1834e Compare August 2, 2022 02:55
@sfc-gh-madkins sfc-gh-madkins mentioned this pull request Aug 15, 2022
Snowflake type behavior is strange #2633
Closed
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch from 6b17836 to 71ade2e Compare August 15, 2022 06:15
adchia
adchia reviewed Aug 15, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/materialization/snowflake_engine.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 2 times, most recently from 456a520 to b42c452 Compare August 16, 2022 04:52
@sfc-gh-madkins sfc-gh-madkins added the do-not-merge/work-in-progress label Aug 16, 2022
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 6 times, most recently from a0b28b7 to ff3c697 Compare August 17, 2022 17:43
achals
achals reviewed Aug 17, 2022
View reviewed changes
Comment thread sdk/python/feast/infra/offline_stores/offline_store.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread sdk/python/feast/infra/online_stores/snowflake.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch 4 times, most recently from 7d99d8b to 70eb0f1 Compare August 18, 2022 04:52
achals
achals approved these changes Aug 18, 2022
View reviewed changes

@achals achals left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@feast-ci-bot feast-ci-bot assigned achals Aug 18, 2022
@feast-ci-bot feast-ci-bot added the lgtm label Aug 18, 2022
@achals

achals commented Aug 18, 2022

Copy link
Copy Markdown
Member

Tests are red because of a couple of tests that aren't being skipped correctly (since pr-local-integration-tests is triggered on pull_request_target).

Going to force merge this in.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch from 70eb0f1 to 8af1077 Compare August 18, 2022 20:33
@feast-ci-bot feast-ci-bot removed the lgtm label Aug 18, 2022
@sfc-gh-madkins
feat: Add Snowflake materialization engine
5d0cf95 
Signed-off-by: Miles Adkins <[email protected]>
@sfc-gh-madkins
sfc-gh-madkins force-pushed the snowflake_materialize branch from 8af1077 to 5d0cf95 Compare August 18, 2022 20:59
achals
achals approved these changes Aug 18, 2022
View reviewed changes

@achals achals left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@feast-ci-bot feast-ci-bot added the lgtm label Aug 18, 2022
@feast-ci-bot

feast-ci-bot commented Aug 18, 2022

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, kevjumba, 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 [achals,kevjumba]

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sfc-gh-madkins sfc-gh-madkins removed the do-not-merge/work-in-progress label Aug 18, 2022
@achals
achals merged commit f3b522b into feast-dev:master Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adchia adchia adchia left review comments
@achals achals achals approved these changes
@kevjumba kevjumba kevjumba approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@achals achals

Labels

approved lgtm ok-to-test size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Snowflake type behavior is strange

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@sfc-gh-madkins @codecov-commenter @kevjumba @achals @feast-ci-bot @adchia
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.