Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SnowflakeSource #776

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add SnowflakeSource #776

wants to merge 1 commit into from

Conversation

philippjfr
Copy link
Member

Adds a SnowflakeSource using the snowflake-connector-python library.

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 115 lines in your changes missing coverage. Please review.

Project coverage is 60.43%. Comparing base (2c83a1b) to head (607dffa).

Files with missing lines Patch % Lines
lumen/sources/snowflake.py 0.00% 115 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #776      +/-   ##
==========================================
- Coverage   60.98%   60.43%   -0.56%     
==========================================
  Files         103      104       +1     
  Lines       12544    12659     +115     
==========================================
  Hits         7650     7650              
- Misses       4894     5009     +115     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:


tables = param.ClassSelector(class_=(list, dict), doc="""
List or dictionary of tables.""")

Copy link
Member

@jbednar jbednar Nov 22, 2024

Choose a reason for hiding this comment

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

Few if any of these parameters seem specific to Snowflake compared to any other authenticated SQL database. I guess it's fine to define them here for now, and move the shared parameters to a base class once support for the next type of authenticated SQL is added? I actually can't detect any code in this class that is specific to Snowflake, so maybe this class is actually just a prototype for AuthenticatedSQLSource, tested on Snowflake so far but potentially usable for other systems as well?

Copy link
Contributor

@ahuang11 ahuang11 Nov 22, 2024

Choose a reason for hiding this comment

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

We might want to consider using SQLAlchemy as the interface to specific SQL engines/dialects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants