You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/10/10 15:52:20 UTC
[GitHub] [airflow] Taragolis opened a new pull request, #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Taragolis opened a new pull request, #26973:
URL: https://github.com/apache/airflow/pull/26973
`TestGetGcpCredentialsAndProjectId.test_disable_logging` time to time failed with selected error
```
____________ TestGetGcpCredentialsAndProjectId.test_disable_logging ____________
self = <tests.providers.google.cloud.utils.test_credentials_provider.TestGetGcpCredentialsAndProjectId testMethod=test_disable_logging>
mock_default = <MagicMock name='from_service_account_file' id='140574344364880'>
mock_info = <MagicMock name='from_service_account_info' id='140574409759184'>
mock_file = <MagicMock name='default' id='140574344364432'>
@mock.patch("google.auth.default", return_value=("CREDENTIALS", "PROJECT_ID"))
@mock.patch(
'google.oauth2.service_account.Credentials.from_service_account_info',
)
@mock.patch(
'google.oauth2.service_account.Credentials.from_service_account_file',
)
def test_disable_logging(self, mock_default, mock_info, mock_file):
# assert not logs
with self.assertLogs(level="DEBUG") as logs:
logging.debug('nothing')
get_credentials_and_project_id(disable_logging=True)
assert logs.output == ['DEBUG:root:nothing']
# assert no debug logs emitted from get_credentials_and_project_id
with self.assertLogs(level="DEBUG") as logs:
logging.debug('nothing')
get_credentials_and_project_id(
keyfile_dict={'private_key': 'PRIVATE_KEY'},
disable_logging=True,
)
assert logs.output == ['DEBUG:root:nothing']
# assert no debug logs emitted from get_credentials_and_project_id
with self.assertLogs(level="DEBUG") as logs:
logging.debug('nothing')
get_credentials_and_project_id(
key_path='KEY.json',
disable_logging=True,
)
> assert logs.output == ['DEBUG:root:nothing']
E AssertionError: assert ['DEBUG:root:...eede10>()]>>'] == ['DEBUG:root:nothing']
E Left contains one more item: 'ERROR:asyncio:Task was destroyed but it is pending!\ntask: <Task pending coro=<<async_generator_asend without __name__>()> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7fda0feede10>()]>>'
E Use -v to get the full diff
tests/providers/google/cloud/utils/test_credentials_provider.py:352: AssertionError
```
Due to the code of `airflow.providers.google.cloud.utils.credentials_provider._CredentialProvider` logging should disable only within the class, so `asyncio` error log not related to disabling log.
Add some workaround for check only logs with specific logger.
--
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
[GitHub] [airflow] Taragolis commented on pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #26973:
URL: https://github.com/apache/airflow/pull/26973#issuecomment-1273518522
cc: @dstandish
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26973:
URL: https://github.com/apache/airflow/pull/26973#discussion_r991528758
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -349,7 +367,7 @@ def test_disable_logging(self, mock_default, mock_info, mock_file):
key_path='KEY.json',
disable_logging=True,
)
- assert logs.output == ['DEBUG:root:nothing']
+ assert_no_logs(logs.records)
Review Comment:
Hey @Taragolis , building off your work, i got inspired, and figured out we could wrap this logic in a context mgr and make it all even cleaner.
No obligation to accept it but here is the code
```suggestion
assert_no_logs(logs.records)
@contextmanager
def assert_no_logs(self, name, level):
with self.assertLogs(level=level) as logs:
logging.debug('nothing')
yield
records = [log_record for log_record in logs.records if log_record.name == name]
if not records:
return
raise AssertionError(f"Did not expect any log message from logger={name!r}, but got: {records}")
```
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26973:
URL: https://github.com/apache/airflow/pull/26973#discussion_r991529302
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -349,7 +367,7 @@ def test_disable_logging(self, mock_default, mock_info, mock_file):
key_path='KEY.json',
disable_logging=True,
)
- assert logs.output == ['DEBUG:root:nothing']
+ assert_no_logs(logs.records)
Review Comment:
then the code becomes this:
```python
# assert no logs
with self.assert_no_logs(name=logger_name, level="DEBUG") as logs:
get_credentials_and_project_id(disable_logging=True)
# assert no debug logs emitted from get_credentials_and_project_id
with self.assert_no_logs(name=logger_name, level="DEBUG") as logs:
get_credentials_and_project_id(
keyfile_dict={'private_key': 'PRIVATE_KEY'},
disable_logging=True,
)
# assert no debug logs emitted from get_credentials_and_project_id
with self.assert_no_logs(name=logger_name, level="DEBUG") as logs:
get_credentials_and_project_id(
key_path='KEY.json',
disable_logging=True,
)
```
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26973:
URL: https://github.com/apache/airflow/pull/26973#discussion_r991496566
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -327,11 +327,28 @@ def test_get_credentials_and_project_id_with_mutually_exclusive_configuration(
'google.oauth2.service_account.Credentials.from_service_account_file',
)
def test_disable_logging(self, mock_default, mock_info, mock_file):
- # assert not logs
+ """
+ Test disable logging in ``get_credentials_and_project_id``.
+
+ Due to selected limitation:
+ - Unable mixin pytest autouse-fixture `caplog` with `unittest.TestCase`
+ - `unittest.TestCase.assertNoLogs` available only in Python 3.10+
+
+ We use some workarounds for filtering specific logger and raise error with these records.
+ """
+ logger_name = "airflow.providers.google.cloud.utils.credentials_provider._CredentialProvider"
+
Review Comment:
small nitty suggestion.... move this line below function def so it's closer to where it's used / easier to see and find
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -327,11 +327,28 @@ def test_get_credentials_and_project_id_with_mutually_exclusive_configuration(
'google.oauth2.service_account.Credentials.from_service_account_file',
)
def test_disable_logging(self, mock_default, mock_info, mock_file):
- # assert not logs
+ """
+ Test disable logging in ``get_credentials_and_project_id``.
+
+ Due to selected limitation:
+ - Unable mixin pytest autouse-fixture `caplog` with `unittest.TestCase`
+ - `unittest.TestCase.assertNoLogs` available only in Python 3.10+
+
+ We use some workarounds for filtering specific logger and raise error with these records.
+ """
+ logger_name = "airflow.providers.google.cloud.utils.credentials_provider._CredentialProvider"
+
+ def assert_no_logs(records, name):
+ records = [log_record for log_record in records if log_record.name == name]
+ if not records:
+ return
+ raise AssertionError(f"Not expected get any log message from logger={name!r}, got: {records}")
Review Comment:
```suggestion
raise AssertionError(f"Did not expect any log message from logger={name!r}, but got: {records}")
```
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -327,11 +327,28 @@ def test_get_credentials_and_project_id_with_mutually_exclusive_configuration(
'google.oauth2.service_account.Credentials.from_service_account_file',
)
def test_disable_logging(self, mock_default, mock_info, mock_file):
- # assert not logs
+ """
+ Test disable logging in ``get_credentials_and_project_id``.
+
+ Due to selected limitation:
+ - Unable mixin pytest autouse-fixture `caplog` with `unittest.TestCase`
+ - `unittest.TestCase.assertNoLogs` available only in Python 3.10+
+
+ We use some workarounds for filtering specific logger and raise error with these records.
Review Comment:
```suggestion
Due to following limitations, we use some workarounds for filtering specific logger and raise error with these records:
- Cannot use pytest autouse-fixture `caplog` with `unittest.TestCase`
- `unittest.TestCase.assertNoLogs` available only in Python 3.10+
```
--
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
[GitHub] [airflow] dstandish merged pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
dstandish merged PR #26973:
URL: https://github.com/apache/airflow/pull/26973
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #26973:
URL: https://github.com/apache/airflow/pull/26973#discussion_r991508315
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -327,11 +327,28 @@ def test_get_credentials_and_project_id_with_mutually_exclusive_configuration(
'google.oauth2.service_account.Credentials.from_service_account_file',
)
def test_disable_logging(self, mock_default, mock_info, mock_file):
- # assert not logs
+ """
+ Test disable logging in ``get_credentials_and_project_id``.
+
+ Due to selected limitation:
+ - Unable mixin pytest autouse-fixture `caplog` with `unittest.TestCase`
+ - `unittest.TestCase.assertNoLogs` available only in Python 3.10+
+
+ We use some workarounds for filtering specific logger and raise error with these records.
+ """
+ logger_name = "airflow.providers.google.cloud.utils.credentials_provider._CredentialProvider"
+
Review Comment:
Good idea. Initially I keep it outside for use it with pure `pytest`.
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #26973: Filter logs for specific logger in `TestGetGcpCredentialsAndProjectId.test_disable_logging`
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26973:
URL: https://github.com/apache/airflow/pull/26973#discussion_r991528758
##########
tests/providers/google/cloud/utils/test_credentials_provider.py:
##########
@@ -349,7 +367,7 @@ def test_disable_logging(self, mock_default, mock_info, mock_file):
key_path='KEY.json',
disable_logging=True,
)
- assert logs.output == ['DEBUG:root:nothing']
+ assert_no_logs(logs.records)
Review Comment:
Hey @Taragolis , building off your work, i got inspired, and figured out we could wrap this logic in a context mgr and make it all even cleaner.
No obligation to accept it but here is the code
```suggestion
assert_no_logs(logs.records)
@contextmanager
def assert_no_logs(self, name, level):
with self.assertLogs(level=level) as logs:
logging.log(level=level, msg='nothing')
yield
records = [log_record for log_record in logs.records if log_record.name == name]
if not records:
return
raise AssertionError(f"Did not expect any log message from logger={name!r}, but got: {records}")
```
--
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