Skip to content

PECOBLR-86 improve logging on python driver #556

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 5 commits into
base: main
Choose a base branch
from

Conversation

saishreeeee
Copy link
Collaborator

@saishreeeee saishreeeee commented May 15, 2025

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • Other

Description

Improve logging on python driver

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually
  • N/A

Related Tickets & Documents

PECOBLR-86

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@@ -214,6 +214,8 @@ def read(self) -> Optional[OAuthToken]:
# use_cloud_fetch
# Enable use of cloud fetch to extract large query results in parallel via cloud storage

logger.debug(f"Connection.__init__(server_hostname={server_hostname}, http_path={http_path})")
Copy link
Contributor

Choose a reason for hiding this comment

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

try to use lazy logging

@@ -390,6 +392,8 @@ def attempt_request(attempt):

# TODO: don't use exception handling for GOS polling...

logger.debug(f"ThriftBackend.attempt_request: HTTPError: {err}")
Copy link
Contributor

Choose a reason for hiding this comment

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

When handling an error, try to log error and not debug

@@ -404,6 +408,7 @@ def attempt_request(attempt):
else:
raise err
except OSError as err:
logger.debug(f"ThriftBackend.attempt_request: OSError: {err}")
Copy link
Contributor

Choose a reason for hiding this comment

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

same, for error cases preferrable to use logger.error

@jprakash-db
Copy link
Contributor

can you look into the failing tests as well

@jprakash-db
Copy link
Contributor

Also can you update the PR description

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@saishreeeee saishreeeee force-pushed the PECOBLR-86-improve-logging-on-python-driver branch from 2485a22 to 0b04de7 Compare May 15, 2025 12:12
Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee saishreeeee force-pushed the PECOBLR-86-improve-logging-on-python-driver branch from 0b04de7 to 0adfedd Compare May 15, 2025 12:15
Signed-off-by: Sai Shree Pradhan <[email protected]>
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee saishreeeee deployed to azure-prod May 15, 2025 12:57 — with GitHub Actions Active
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.

2 participants