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

Allow enabling DEBUG logs for databricks-sql-connector #7860

Closed

Conversation

susodapop
Copy link

@susodapop susodapop commented Jun 13, 2023

resolves #7859

Description

This change allows us to turn on DEBUG level logs for databricks-sql-connector when invoked from dbt.

Regarding documentation updates: I'm going to open a PR for dbt-databricks that includes more fine-grained control over the logs from databricks-sql-connector. The doc update for this PR will be included at that time.

Checklist

Jesse Whitehouse added 2 commits June 13, 2023 14:18
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop susodapop requested a review from a team as a code owner June 13, 2023 19:35
@susodapop susodapop requested a review from aranke June 13, 2023 19:35
Copy link
Member

@aranke aranke left a comment

Choose a reason for hiding this comment

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

LGTM!

@aranke aranke closed this Jun 16, 2023
@aranke aranke reopened this Jun 16, 2023
@aranke
Copy link
Member

aranke commented Jun 16, 2023

@susodapop We should be good to merge this, just checking if you have signed the CLA?

@jtcohen6 jtcohen6 linked an issue Jun 16, 2023 that may be closed by this pull request
3 tasks
@dbeatty10
Copy link
Contributor

@aranke would either you or @peterallenwebb (or both!) be willing to take a peek at @JerCo's comment here?

His key question is:

Is there any way to put this logic inside the dbt-databricks adapter instead?

@susodapop
Copy link
Author

We should be good to merge this, just checking if you have signed the CLA?

Yes @aranke I've signed the CLA :)

@dbeatty10 I responded to JerCo's comment in that thread. If this makes more sense in dbt-databricks we can move it there. The existing solution is meant to follow the pattern set for the snowflake connector.

@dbeatty10
Copy link
Contributor

@cla-bot check

@cla-bot cla-bot bot added the cla:yes label Jun 23, 2023
@cla-bot
Copy link

cla-bot bot commented Jun 23, 2023

The cla-bot has been summoned, and re-checked this pull request!

@dbeatty10
Copy link
Contributor

@aranke CLA check is passing now. I'll let you either merge this (or not) depending on the outcome of this discussion: #7859 (comment)

@colin-rogers-dbt
Copy link
Contributor

This won't actually work since it only enables the legacy logger that's being deprecated, see #8643 for inclusion in standard dbt logging

@dbeatty10
Copy link
Contributor

I'm going to close this in favor of #8643 (which has been merged).

We can re-visit this (including re-opening this PR) if needed.

@dbeatty10 dbeatty10 closed this Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2693] [Feature] Allow debug logs for dbt-databricks
4 participants