[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: esadler-hbo
To complete the pull request process, please assign woop after the PR has been reviewed.
You can assign the PR to them by writing /assign @woop in a comment when ready.

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

/assign @woop

I noticed an issue for me, but obviously the quotes could be important for somethings. I was hoping to throw up a PR that would run the python integration test suite.

Also let me know if you have ideas on how to test this more officially than editing the repo locally. I'm sure it is possible, but I am new to this repo!

/ok-to-test

Codecov Report

Merging #2601 (5759f07) into master (c439611) will decrease coverage by 22.46%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #2601       +/-   ##
===========================================
- Coverage   82.30%   59.84%   -22.47%     
===========================================
  Files         155      153        -2     
  Lines       12747    12620      -127     
===========================================
- Hits        10492     7553     -2939     
- Misses       2255     5067     +2812     

Flags with carried forward coverage won't be shown. Click here to find out 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 c439611...5759f07. Read the comment docs.

@sfc-gh-madkins If I remember correctly you added this quite recently, could you take a look?

@esadler-hbo do you mind adding a test or something that reproduces the issue in #2595 without this patch?

would be great to understand what your feature_store.yaml / config file looks like?

would be great to understand what your feature_store.yaml / config file looks like?

I could totally be doing this wrong, but this is what I had. I did not use quotes for any of the filled in values except the config path. Not filling in because I probably shouldn't share.

# feature_store.yaml
project: firm_finch
registry: registry.db
provider: local
offline_store:
    type: snowflake.offline
    account: 
    user: 
    password: 
    role:
    warehouse:
    database:
    schema:
    config_path: "Users/.../config.ini"

The config file

[connections.feast_offline_store]
private_key_passphrase = phrase
private_key = path_to_key

@sfc-gh-madkins If I remember correctly you added this quite recently, could you take a look?

@esadler-hbo do you mind adding a test or something that reproduces the issue in #2595 without this patch?

Happy to add a test once @sfc-gh-madkins lets me know if my config setup makes sense. I just need to get setup with a test environment and then getting some direction to where and how be much appreciated.

my guess is you are not respecting case sensitivity -- most likely your warehouse is ALL CAPS in Snowflake but you are putting it in lowercase in your config file

Currently feast is using double quoted identifier syntax -- see https://docs.snowflake.com/en/sql-reference/identifiers-syntax.html

i appreciate that this is not documented well in the feast docs

my guess is you are not respecting case sensitivity -- most likely your warehouse is ALL CAPS in Snowflake but you are putting it in lowercase in your config file

Currently feast is using double quoted identifier syntax -- see https://docs.snowflake.com/en/sql-reference/identifiers-syntax.html

i appreciate that this is not documented well in the feast docs

Well I reset my environment and I couldn't reproduce the issue. Sorry to bother you all!