feat(SnowFlake): snowflake connector #574
Conversation
WalkthroughThis update introduces new connector configurations for Yahoo Finance and Snowflake, refactors existing SQL connectors, and adds corresponding unit tests. It also includes examples demonstrating how to use these connectors with the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 10
Files ignored due to filter (1)
- pyproject.toml
Files selected for processing (7)
- examples/from_snowflake.py (1 hunks)
- pandasai/connectors/init.py (2 hunks)
- pandasai/connectors/base.py (2 hunks)
- pandasai/connectors/sql.py (6 hunks)
- pandasai/connectors/yahoo_finance.py (3 hunks)
- tests/connectors/test_base.py (2 hunks)
- tests/connectors/test_sql.py (2 hunks)
Files skipped from review due to trivial changes (2)
- pandasai/connectors/init.py
- tests/connectors/test_base.py
Additional comments (Suppressed): 10
tests/connectors/test_sql.py (2)
1-7: The import statement for the configuration class has been updated to reflect the new
SQLConnectorConfigclass. Ensure that this change is reflected throughout the codebase.16-22: The
ConnectorConfiginstance has been replaced with aSQLConnectorConfiginstance. Make sure that all properties required bySQLConnectorConfigare provided and that they are valid.pandasai/connectors/yahoo_finance.py (2)
1-8: The import of
create_enginefromsqlalchemyand the renaming ofConnectorConfigtoYahooFinanceConnectorConfigin line 5 are new additions. Ensure that these changes do not break any existing functionality or dependencies elsewhere in the codebase.23-30: The
YahooFinanceConnectorConfigobject is now being initialized without a username and password, which were present in the old hunk. If the Yahoo Finance API requires authentication, this could be an issue. Please verify if these credentials are necessary for the connection.pandasai/connectors/sql.py (6)
27-28: The
__init__method of theSQLConnectorclass now accepts a union type for theconfigparameter, which can be either aBaseConnectorConfigobject or a dictionary. This change provides more flexibility in how the configuration is passed to the connector. However, it's important to ensure that all calls to this method throughout the codebase have been updated to match the new signature.34-35: The
configobject is loaded using the_load_connector_configmethod before being passed to the parent class constructor. This approach allows for additional processing of the configuration before it's used to initialize the base class. However, it's crucial to verify that the_load_connector_configmethod correctly handles bothBaseConnectorConfigobjects and dictionaries.365-366: The
__init__method of theMySQLConnectorclass now accepts aSQLConnectorConfigobject instead of aConnectorConfigobject. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.394-395: The
__init__method of thePostgreSQLConnectorclass now accepts aSQLConnectorConfigobject instead of aConnectorConfigobject. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.445-466: The
__init__method of theSnowFlakeConnectorclass sets default values for various configuration parameters using environment variables if they're not provided in theconfigobject. This approach provides flexibility and allows sensitive information like usernames and passwords to be stored securely in environment variables. However, it's important to ensure that these environment variables are set correctly in the deployment environment.486-506: The
headmethod of theSnowFlakeConnectorclass is decorated with thecachedecorator from thefunctoolsmodule. This decorator caches the result of the method and returns the cached result on subsequent calls with the same arguments. This can significantly improve performance when retrieving the head of the data source multiple times. However, it's important to consider the implications of caching in terms of memory usage and data freshness.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files selected for processing (2)
- pandasai/connectors/base.py (2 hunks)
- pandasai/connectors/yahoo_finance.py (3 hunks)
Additional comments (Suppressed): 3
pandasai/connectors/yahoo_finance.py (3)
1-7: The import of the
YahooFinanceConnectorConfigclass has been updated to reflect the new class name. This change is consistent with the PR summary and seems to be correctly implemented.22-29: The instantiation of the
YahooFinanceConnectorConfigobject has been updated to remove theusernameandpasswordparameters, which are not needed for Yahoo Finance. This change improves security by not storing unnecessary credentials.33-36: The initialization of the
YahooFinanceConnectorclass remains largely unchanged, except for the use of the newYahooFinanceConnectorConfigclass. The changes are consistent with the PR summary and seem to be correctly implemented.
examples/from_snowflake.py
Outdated
|
|
||
| OPEN_AI_API = "Your-API-Key" | ||
| llm = OpenAI(api_token=OPEN_AI_API) | ||
| df = SmartDatalake([snowflake_connector], config={"llm": llm}) |
There was a problem hiding this comment.
Guess since it's a single connector, we can use the SmartDatframe here instead, like this:
df = SmartDatframe(snowflake_connector, config={"llm": llm})
| @@ -34,7 +33,7 @@ def __init__(self, stock_ticker, where=None, cache_interval: int = 600): | |||
| self._cache_interval = cache_interval | |||
| super().__init__(yahoo_finance_config) | |||
| self.ticker = yfinance.Ticker(self._config.table) | |||
|
|
|||
|
WIP |
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 0 minutes and 17 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #574 +/- ##
==========================================
- Coverage 83.32% 83.28% -0.04%
==========================================
Files 53 53
Lines 2602 2651 +49
==========================================
+ Hits 2168 2208 +40
- Misses 434 443 +9
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 14
Files selected for processing (5)
- examples/from_snowflake.py (1 hunks)
- examples/from_yahoo_finance.py (1 hunks)
- pandasai/connectors/base.py (2 hunks)
- pandasai/connectors/sql.py (6 hunks)
- tests/connectors/test_snowflake.py (1 hunks)
Additional comments (Suppressed): 9
tests/connectors/test_snowflake.py (2)
22-24: The values for
username,password, andaccountare placeholders ("your_username", "your_password", "ehxzojy-ue47135"). Ensure that these are replaced with valid credentials during testing. If these are meant to be placeholders, consider making it more explicit, e.g., "YOUR_USERNAME_HERE".51-58: The
_build_querymethod is being tested here, but it's not clear whether SQL injection attacks have been considered. Ensure that the query building process is safe from SQL injection attacks by using parameterized queries or other secure practices.pandasai/connectors/base.py (1)
- 75-90: The
_load_connector_config()and_init_connection()methods have been added but they don't contain any implementation. If these methods are intended to be abstract and implemented by subclasses, they should be decorated with the@abstractmethoddecorator. Otherwise, they should include some basic implementation that can be extended or overridden by subclasses.pandasai/connectors/sql.py (6)
8-12: The import statements have been updated to include the new
SnowFlakeConnectorConfigclass and the refactoredBaseConnectorConfigclass. Ensure that these classes are correctly implemented and available in the.basemodule.38-39: The
_load_connector_config()method is called to process theconfigargument. This method should handle bothBaseConnectorConfigobjects and dictionaries, converting them into aSQLConnectorConfigobject. Ensure that this method is correctly implemented and handles all possible types and formats of theconfigargument.44-45: The
_init_connection()method is called to initialize the database connection using theconfigargument. Ensure that this method is correctly implemented and establishes a secure and reliable connection to the database.46-47: The cache interval is set after the connection has been initialized. This order of operations seems logical, but make sure that the cache interval does not need to be set before the connection is established for any reason.
58-59: The
_load_connector_config()method is expected to convert theconfigargument into aSQLConnectorConfigobject. Make sure that this method correctly handles all possible types and formats of theconfigargument.446-528: The new
SnowFlakeConnectorclass extends theSQLConnectorclass and provides functionality for connecting to Snowflake databases. It overrides the_load_connector_config(),_init_connection(),head(), and__repr__()methods, and adds additional configuration properties specific to Snowflake databases. Ensure that these methods are correctly implemented and provide the expected functionality.
gventuri
changed the title
* Add Basic code for SnowFlake Connector * feat[Snowflake]: Adding SnowFlake Connector * fix: missing , in where clause of example * test: snowflake parser improvements * fix: Yahoo connector * fix: ruff issues * fix: example of yahoo finance * Adding test cases for snowflake * fix doc string
Connects to snowflake data source by providing credentials
Summary by CodeRabbit
SnowFlakeConnectorclass.SQLConnectorConfig.YahooFinanceConnectorConfigto provide a more tailored configuration for Yahoo Finance connections.ConnectorConfigtoBaseConnectorConfigand introduced derived classes for specific connectors.SnowFlakeConnectorclass.