Skip to content

Commit

Permalink
fix: Fix "Connection is closed" error when using browser authenticati…
Browse files Browse the repository at this point in the history
…on (#257)

The purpose of this PR is to solve the issue reported
[here](#225).

## Root cause:

There was one usage of `engine.connect` outside of a context manager (in
`create_engine`), causing the connector to leave one connection "lying
around".

I did not dive too deep into the internals of the connector nor focused
too much in understanding why is it different for the browser
authentication mechanism, but mostly in ensuring all calls to
`engine.connect()` were done within a context manager, ensuring SQL
alchemy does its job in closing all connections when they should be
closed.

## Implementation details:

- Use a context manager when checking if the database exists (in the
`create_engine` ) method.
  • Loading branch information
dlouseiro authored Sep 16, 2024
1 parent 82f5081 commit c6c6305
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions target_snowflake/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ def create_engine(self) -> Engine:
connect_args=connect_args,
echo=False,
)
connection = engine.connect()
db_names = [db[1] for db in connection.execute(text("SHOW DATABASES;")).fetchall()]
if self.config["database"] not in db_names:
msg = f"Database '{self.config['database']}' does not exist or the user/role doesn't have access to it."
raise Exception(msg) # noqa: TRY002
with engine.connect() as conn:
db_names = [db[1] for db in conn.execute(text("SHOW DATABASES;")).fetchall()]
if self.config["database"] not in db_names:
msg = f"Database '{self.config['database']}' does not exist or the user/role doesn't have access to it."
raise Exception(msg) # noqa: TRY002
return engine

def prepare_column(
Expand Down

0 comments on commit c6c6305

Please sign in to comment.