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.

Sign up for GitHub

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

Jump to bottom

feat: add support to log commit stats #205

Merged
merged 9 commits into from Feb 23, 2021
Merged

feat: add support to log commit stats #205

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a5a680c
feat: add support for logging commit stats
larkee Jan 5, 2021
d6e6db4
test: add commit stats to CommitResponse
larkee Jan 28, 2021
6b97f72
style: fix lint errors
larkee Jan 28, 2021
ca867a8
refactor: remove log formatting
larkee Feb 18, 2021
b2bf238
test: update info arg assertions
larkee Feb 18, 2021
507a4ec
docs: document logger param
larkee Feb 19, 2021
11f62bc
refactor: pass CommitStats via extra kwarg
larkee Feb 19, 2021
3dae32a
fix: ensure logger is unused if commit fails
larkee Feb 22, 2021
06182d2
Merge branch 'master' into commit-stats
larkee Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 1 addition & 11 deletions google/cloud/spanner_v1/database.py
View file
Open in desktop
Expand Up @@ -237,12 +237,6 @@ def logger(self):

ch = logging.StreamHandler()
ch.setLevel(logging.INFO)

formatter = logging.Formatter(
"%(asctime)s - %(name)s - %(levelname)s - %(message)s"
)
ch.setFormatter(formatter)

self._logger.addHandler(ch)
return self._logger

Expand Down Expand Up @@ -657,11 +651,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
self._batch.commit(return_commit_stats=self._database.log_commit_stats)
finally:
if self._database.log_commit_stats:
self._database.logger.info(
"Transaction mutation count: {}".format(
self._batch.commit_stats.mutation_count
)
)
self._database.logger.info(self._batch.commit_stats)
self._database._pool.put(self._session)


Expand Down
6 changes: 1 addition & 5 deletions google/cloud/spanner_v1/session.py
View file
Open in desktop
Expand Up @@ -358,11 +358,7 @@ def run_in_transaction(self, func, *args, **kw):
raise
else:
if self._database.log_commit_stats:
self._database.logger.info(
"Transaction mutation count: {}".format(
txn.commit_stats.mutation_count
)
)
self._database.logger.info(txn.commit_stats)
Copy link

Choose a reason for hiding this comment

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

I wnet over this in my other comment, but I'll repeat it here too: The docs for Python Logging say they expect the main msg argument to be a string. If you want to add a custom object, see the extra argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's exactly what I want. I've updated the call to use a formatted message and pass the CommitStats via the extra argument. Thank you for your patience while I learn how to use loggers correctly 👍

Copy link

Choose a reason for hiding this comment

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

No problem! Yeah this now LGTM from a logging perspective

return return_value


Expand Down