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

DbAPiHook: Don't log a warning message if placeholder is None and make sure warning message is formatted correctly #39690

Merged
merged 8 commits into from
May 18, 2024
17 changes: 9 additions & 8 deletions airflow/providers/common/sql/hooks/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,15 @@ def __init__(self, *args, schema: str | None = None, log_sql: bool = True, **kwa
def placeholder(self):
conn = self.get_connection(getattr(self, self.conn_name_attr))
placeholder = conn.extra_dejson.get("placeholder")
if placeholder in SQL_PLACEHOLDERS:
return placeholder
self.log.warning(
"Placeholder defined in Connection '%s' is not listed in 'DEFAULT_SQL_PLACEHOLDERS' "
"and got ignored. Falling back to the default placeholder '%s'.",
placeholder,
self._placeholder,
)
if placeholder:
if placeholder in SQL_PLACEHOLDERS:
return placeholder
self.log.warning(
"Placeholder defined in Connection '%s' is not listed in 'DEFAULT_SQL_PLACEHOLDERS' "
"and got ignored. Falling back to the default placeholder '%s'.",
self.conn_name_attr,
self._placeholder,
)
return self._placeholder

def get_conn(self):
Expand Down
50 changes: 50 additions & 0 deletions tests/providers/common/sql/hooks/test_dbapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,56 @@ def test_get_uri_without_auth_and_empty_host(self):
)
assert self.db_hook.get_uri() == "conn-type://@:3306/schema?charset=utf-8"

def test_placeholder(self):
self.db_hook.get_connection = mock.MagicMock(
return_value=Connection(
conn_type="conn-type",
login=None,
password=None,
schema="schema",
port=3306,
)
)
assert self.db_hook.placeholder == "%s"

self.db_hook.log.warning.assert_not_called()

def test_placeholder_with_valid_placeholder_in_extra(self):
self.db_hook.get_connection = mock.MagicMock(
return_value=Connection(
conn_type="conn-type",
login=None,
password=None,
schema="schema",
port=3306,
extra=json.dumps({"placeholder": "?"}),
)
)
assert self.db_hook.placeholder == "?"

self.db_hook.log.warning.assert_not_called()

def test_placeholder_with_invalid_placeholder_in_extra(self):
self.db_hook.get_connection = mock.MagicMock(
return_value=Connection(
conn_type="conn-type",
login=None,
password=None,
schema="schema",
port=3306,
extra=json.dumps({"placeholder": "!"}),
)
)

assert self.db_hook.placeholder == "%s"

self.db_hook.log.warning.assert_called_once_with(
"Placeholder defined in Connection '%s' is not listed in 'DEFAULT_SQL_PLACEHOLDERS' "
"and got ignored. Falling back to the default placeholder '%s'.",
"test_conn_id",
self.db_hook._placeholder,
)

def test_run_log(self):
statement = "SQL"
self.db_hook.run(statement)
Expand Down