[GitHub] [airflow] josh-fell commented on a diff in pull request #24933: Apply flake8-logging-format changes to providers

[GitBox <gi...@apache.org>]

josh-fell commented on code in PR #24933:
URL: https://github.com/apache/airflow/pull/24933#discussion_r934605742


##########
tests/providers/amazon/aws/log/test_s3_task_handler.py:
##########
@@ -107,9 +106,8 @@ def test_hook_raises(self):
 
             mock_error.assert_called_once_with(
                 'Could not create an S3Hook with connection id "%s". Please make '
-                'sure that apache-airflow[aws] is installed and the S3 connection exists. Exception : "%s"',

Review Comment:
   This an example of where I'm unsure if accounting for `G200` error (Logging statements should not include the exception in logged string (use exception or exc_info=True) will negatively affect what's logged and, I suppose more importantly, what the log handlers will ingest. 
   
   Here the "Exception: <exception message>" is part of the logging line directly. But does this matter when `exc_info=True` or when there is a following `raise`? Is there really any lost information in the logs?



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org