o-nikolas commented on code in PR #76639:
URL: https://github.com/apache/airflow/pull/76639#discussion_r4517227410
##########
providers/amazon/tests/unit/amazon/aws/hooks/test_duckdb.py:
##########
@@ -138,6 +140,35 @@ def test_credential_chain_is_the_default(self,
aws_base_hook):
assert "PROVIDER credential_chain" in statement
assert "KEY_ID" not in statement
+ @pytest.mark.parametrize(
+ ("aws_conn", "hook_kwargs", "warns"),
+ [
+ pytest.param(AWS_KEYS, {}, True, id="keys"),
+ pytest.param({"extra": '{"aws_session_token": "t"}'}, {}, True,
id="session-token"),
+ pytest.param({"extra": '{"role_arn": "arn:aws:iam::1:role/r"}'},
{}, True, id="role"),
+ pytest.param({"extra": '{"profile_name": "p"}'}, {}, True,
id="profile"),
+ pytest.param({"extra": '{"region_name": "eu-west-1"}'}, {}, False,
id="no-credentials"),
+ pytest.param(None, {}, False, id="missing-connection"),
+ pytest.param(AWS_KEYS, {"aws_conn_id": None}, False,
id="no-aws-conn"),
+ pytest.param(AWS_KEYS, {"credential_strategy":
"credential_chain"}, False, id="chain"),
+ pytest.param(AWS_KEYS, {"credential_strategy": "config"}, False,
id="config"),
+ ],
+ )
+ def test_warns_when_the_aws_connection_credentials_are_not_used(
+ self, aws_base_hook, aws_conn, hook_kwargs, warns
+ ):
+ conn = Connection(conn_id="aws_default", conn_type="aws", **aws_conn)
if aws_conn else None
+ aws_base_hook.conn_config = AwsConnectionWrapper(conn)
+ with mock.patch.object(AwsDuckDBHook, "log") as mock_log:
+ statement = secret_statement(AwsDuckDBHook(**hook_kwargs))
+ warnings = [c for c in mock_log.warning.call_args_list if "own
credential chain" in c.args[0]]
Review Comment:
nit: filtering on the message text is close to asserting on raw log text,
which AGENTS.md asks us to avoid. If nothing else warns here, could this assert
on `mock_log.warning.call_args_list` directly?
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/duckdb.py:
##########
@@ -210,6 +213,7 @@ def configure_secrets(self, conn: DuckDBPyConnection) ->
None:
self.log.debug("credential_strategy is 'none'; not creating a
DuckDB S3 secret.")
return
+ self._warn_if_connection_credentials_are_ignored()
Review Comment:
`_log_aws_identity` below, runs once per hook, but this warning repeats on
every `get_conn()`. I think once per hook in the same way makes sense for this
warning.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
