Conversation
…actor and enhanced session management - Moved execution logic from Snowflake connector to improve control and separation of concerns. - Introduced a dedicated class hierarchy for SQL statement handling. - Enhanced for better session state management and query tagging. - Created class for managing change history operations. - Improved SQL statement parsing and error handling. - Updated version in setup.cfg and CHANGELOG.md to reflect upcoming release.
… retrieval - Added logic to skip execution of empty SQL statements in the Script class. - Improved session state and query tag retrieval by switching from async to simple execution in the SnowflakeSession class. - Consolidated reset query execution into a single anonymous block for efficiency.
…xecution - Enhanced the Script class to manage transactions by rolling back on statement failures or exceptions. - Updated the execute_snowflake_query method in SnowflakeSession to support explicit auto-commit control. - Improved logging for transaction outcomes and error reporting during script execution.
schemachange/session/Script.py
Outdated
| def sql(self) -> str: | ||
| """Return the SQL to execute (without comments for non-empty, with comments for empty)""" | ||
| return ( | ||
| self.sql_without_comments if not self.is_empty else self.sql_with_comments |
There was a problem hiding this comment.
hmm this should return the sql with comments.
schemachange/session/Script.py
Outdated
| logger: structlog.BoundLogger, | ||
| ) -> ExecutionResult: | ||
| """Execute a single statement and return the result""" | ||
| import time |
… creation - Removed direct instantiation of HistorySession in SnowflakeSession, replacing it with a lazy initialization approach. - Added a new property to fetch change history metadata, enhancing the separation of concerns. - Updated tests to mock HistorySession behavior for improved isolation and reliability.
…ed sql. tests adjusted to reflect. consolidated imports.
|
@MACKAT05 Could we revisit these changes in a future release. There will be a release 4.1.0 that has been focused on supporting the cadence of arguments from CLI > ENV > Config YAML > connections.toml that will be released soon. Could we meet to discuss this change and see where we can table this in the roadmap to 4.2.0, 4.3.0, 4.4.0, 4.5.0 or 5.0.0? |
sfc-gh-tmathew
added
Under Review
|
Hi @MACKAT05, Thank you for this architectural refactor! This is a significant change that moves execution logic into the package and introduces new A few questions before we proceed:
This PR has conflicts - please hold off on rebasing until we align on direction. Thanks for your patience! |
This removes a big capability issue. The connector method that was used to submit scripts handled parseing and splitting statements and was the source of a number of the "weird" behaviors that have been complained about. reading the code on the python connector it is also marked as not for production or some such verbiage as it was provided as a convience method. Moving the parsing responsibility to this tool ensures we can fix issues in the future ( like correctly submitting code when there were trailing comments or SQL stored proceduredures where there are semi colons in series.)
Large scripts or large sets. No to both. From what I recall the parsing was mostly the same flow and method wise. With additional steps currently to resolve send the data over the wire. I think it ends up loading the data twice and compares it but i suspect that parsing may not be impacted much since we are reading from memory instead of disk. agree we need to test this though.
|
2 participants
This pull request introduces a major architectural refactor to the project, focusing on improving script execution, session and history management, and error handling. The execution logic has been moved from the Snowflake connector directly into this package, enabling more robust control and separation of concerns. A new
Scriptclass hierarchy now handles parsing and execution of individual SQL statements, and dedicated session classes manage state and history. SQL parsing and error reporting are also significantly enhanced.Architectural Refactor and Session Management
CHANGELOG.md, CHANGELOG.mdR6-R15)Script Execution and Parsing
Scriptclass hierarchy (Script.py) that parses scripts into individual statements with context, executes each statement, and provides detailed execution reports with statement-level success/failure and error tracking. (Script.py, [1] [2]Script.py, schemachange/session/Script.pyR183-R413)Session and History Management
HistorySessionclass for managing change history table operations, including schema/table creation, metadata fetching, and script history retrieval. (HistorySession.py, schemachange/session/HistorySession.pyR1-R271)SnowflakeSessionclass (referenced in new code) to manage session state, query tags, and execution routing. (CHANGELOG.md, CHANGELOG.mdR6-R15)Error Handling and Reporting
Script.py, schemachange/session/Script.pyR183-R413)Script.py, schemachange/session/Script.pyR6-R147)Miscellaneous
4.X.Xincli.pyto reflect the major changes. (cli.py, schemachange/cli.pyL16-R16)demo/basics_demo/2_test/V1.0.1__EOF_FIle.sql, demo/basics_demo/2_test/V1.0.1__EOF_FIle.sqlL8-R8)note: the change in the session pattern does create a second login call which may annoy or frustrate users who do not have MFA caching enabled properly...