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

Compress to one connection manager #910

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Compress to one connection manager #910

merged 2 commits into from
Jan 23, 2025

Conversation

benc-db
Copy link
Collaborator

@benc-db benc-db commented Jan 21, 2025

Description

For a long time we've had code that suggested that you could either reuse sessions or not by setting an env variable. This hasn't worked for a long time, and since I need to refactor some of this code to improve cancellation behavior, I want to first compress to a single ConnectionManager, so that I don't need to make changes against both. I eventually want to get rid of session reuse, but its too large to do as part of 1.9.x, where I want to improve cancellation behavior, so for now, let the code include only the paths that are actually reachable.

Note: no new code here, just collapsing into the parent class, and keeping the union of the code from the child and the parent. Each method comes either from the parent (if the previous child didn't override) or the child (if the child did override, or added a new method).

Checklist

  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

if conn and conn.name == conn_name and conn.state == ConnectionState.OPEN:
# Found a connection and nothing to do, so just return it
return conn
conn = self._get_if_exists_compute_connection(_get_compute_name(query_header_context) or "")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Git is representing this as a bunch of changes here, but the code is not net new, just moved up into the parent class.

@benc-db benc-db merged commit 395801e into main Jan 23, 2025
6 checks passed
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