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

[BUGFIX] 0.18.4 - Fix Snowflake error when using alternative connection details format. #9056

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

Kilo59
Copy link
Contributor

@Kilo59 Kilo59 commented Dec 6, 2023

Copy of the V1 fix so that we can release as part of 0.18.4

Changes the behavior of SnowflakeDatasource.get_execution_engine() so that it no longer re-creates the sqlalchemy.engine.Engine object and instead re-uses the one used by the Datasource for .test_connection().

This solves 2 problems...

  1. Reduces waste and extra connections caused by having 2 different sqlalchemy.engine.Engine objects at runtime.
  2. Removes the problem of having slightly different configured sqlalchemy.engine.Engine objects.
    • This is the source of the bug that requires a new OSS release.

This is a more minimal version of the following bugfix. Issues with SQLite are forcing the more minimal PR (9055).

Other notes

For Snowflake this also means that when using the authenticator=externalbrowser query param, the browser will only need to auth once instead of 2wice for every connection.

@Kilo59 Kilo59 self-assigned this Dec 6, 2023
@Kilo59 Kilo59 marked this pull request as ready for review December 6, 2023 20:37
@Kilo59 Kilo59 changed the title [BUGFIX] 0.18.4 - [BUGFIX] 0.18.4 - Fix Snowflake error when using alternative connection details format. Dec 6, 2023
@ghost
Copy link

ghost commented Dec 6, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@Kilo59 Kilo59 merged commit 26d2018 into 0.18.x Dec 6, 2023
48 of 54 checks passed
@Kilo59 Kilo59 deleted the b/lakitu-513/18_4 branch December 6, 2023 21:09
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.

1 participant