Skip to content

Conversation

@holly-evans
Copy link
Contributor

closes: #11618

DbApiHook accepts log_sql kwarg to turn off logging the SQL query. In BaseSQLOperator, hook_kwargs flows through to the hook, including log_sql.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Some user documentation would be handy,

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

user docs missing

@holly-evans
Copy link
Contributor Author

@potiuk updated!

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@holly-evans
Copy link
Contributor Author

@uranusjr updated

return conn

self.db_hook = UnitTestDbApiHook()
self.db_hook_log_sql = UnitTestDbApiHook(log_sql=False)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.db_hook_log_sql = UnitTestDbApiHook(log_sql=False)
self.db_hook_no_log_sql = UnitTestDbApiHook(log_sql=False)

This should be clearer? But since this is only used by one test, it’s better to simply instantiate the hook in that test instead.

Copy link
Contributor Author

@holly-evans holly-evans Jun 23, 2022

Choose a reason for hiding this comment

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

@uranusjr Sure, I can rename it. I instantiate the hook here because UnitTestDbApiHook is defined here, I could pull it out perhaps? Also db_hook_schema_override is only used in one test and defined here, so I kept that pattern.

Copy link
Member

Choose a reason for hiding this comment

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

A lot of the provider tests are historically suboptimally written, they don’t always serve as good style guides 🙂

holly-evans and others added 2 commits June 23, 2022 13:26
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@potiuk potiuk merged commit 53284cf into apache:main Jul 4, 2022
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:new-feature Changelog: New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support to turn off the sql echo at logging in dbapi_hook

4 participants