Skip to content

Don't use the global default logging instance when logging #988

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jun 11, 2025

What changes are proposed in this pull request?

The Python SDK sometimes uses the global logging instance to log rather than a per-module logger instance, e.g.

logging.debug(f"Failed to initialize globals 'sqlContext' and 'table', continuing. Cause: {e}")

Using logging.debug configures a root logger and is considered a bad practice when using the logging library in Python. This can lead to several issues including duplicated logs.

This PR makes this change for the few places where the default logger instance is used in the SDK.

How is this tested?

Describe any tests you have done; especially if test tests are not part of
the unit tests (e.g. local tests).

ALWAYS ANSWER THIS QUESTION: Answer with "N/A" if tests are not applicable
to your PR (e.g. if the PR only modifies comments). Do not be afraid of
answering "Not tested" if the PR has not been tested. Being clear about what
has been done and not done provides important context to the reviewers.

mgyucht added 2 commits June 11, 2025 14:35
# Conflicts:
#	databricks/sdk/credentials_provider.py
@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:41 — with GitHub Actions Inactive
@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:42 — with GitHub Actions Inactive
@mgyucht mgyucht changed the title No default logging Don't use the global default logging instance when logging Jun 11, 2025
@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:43 — with GitHub Actions Inactive
@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:43 — with GitHub Actions Inactive
@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:43 — with GitHub Actions Inactive
@@ -3,7 +3,7 @@
import logging
from typing import Dict, Optional, Union, cast

logger = logging.getLogger("databricks.sdk")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This existed before, but wasn't consistently used. I've renamed it to _LOG as it is not meant to be exposed publicly and it is consistent with other places within the SDK.

Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-py

Inputs:

  • PR number: 988
  • Commit SHA: 56487bbd0c808cf046dad7fc96f31556e8c04dae

Checks will be approved automatically on success.

@mgyucht mgyucht temporarily deployed to test-trigger-is June 11, 2025 14:44 — with GitHub Actions Inactive
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