kosteev commented on code in PR #33500:
URL: https://github.com/apache/airflow/pull/33500#discussion_r1301156996
##########
tests/providers/cncf/kubernetes/utils/test_pod_manager.py:
##########
@@ -252,6 +253,26 @@ def test_parse_log_line(self):
assert timestamp == pendulum.parse(real_timestamp)
assert line == log_message
+
@mock.patch("airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager.read_pod_logs")
+ def test_fetch_container_logs_returning_last_timestamp(self,
mock_read_pod_logs):
+ timestamp_string = "2020-10-08T14:16:17.793417674Z"
+
+ mock_read_pod_logs.return_value = [bytes(f"{timestamp_string}
message", "utf-8"), b"notimestamp"]
Review Comment:
Nit: seems that new line above can be omit. Given that there are too many
new lines in the method, I would strive to reduce them if possible to make code
more logically grouped for better readability.
##########
tests/providers/cncf/kubernetes/utils/test_pod_manager.py:
##########
@@ -252,6 +253,26 @@ def test_parse_log_line(self):
assert timestamp == pendulum.parse(real_timestamp)
assert line == log_message
+
@mock.patch("airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager.read_pod_logs")
+ def test_fetch_container_logs_returning_last_timestamp(self,
mock_read_pod_logs):
+ timestamp_string = "2020-10-08T14:16:17.793417674Z"
+
+ mock_read_pod_logs.return_value = [bytes(f"{timestamp_string}
message", "utf-8"), b"notimestamp"]
+
+ self.first_time = True
+
+ def mock_container_is_running(pod, container_name):
+ if self.first_time:
+ self.first_time = False
+ return True
+ return False
+
+ self.pod_manager.container_is_running = mock_container_is_running
Review Comment:
This has to be done with mock.patch, otherwise it will be preserved for next
tests.
##########
tests/providers/cncf/kubernetes/utils/test_pod_manager.py:
##########
@@ -252,6 +253,26 @@ def test_parse_log_line(self):
assert timestamp == pendulum.parse(real_timestamp)
assert line == log_message
+
@mock.patch("airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager.read_pod_logs")
+ def test_fetch_container_logs_returning_last_timestamp(self,
mock_read_pod_logs):
+ timestamp_string = "2020-10-08T14:16:17.793417674Z"
+
+ mock_read_pod_logs.return_value = [bytes(f"{timestamp_string}
message", "utf-8"), b"notimestamp"]
+
+ self.first_time = True
Review Comment:
This looks a bit clumsy to define here self variable that is used only in
this test.
I can suggest this approach:
```
def mock_container_is_running(...):
if mock_container_is_running.first_time:
...
...
mock_container_is_running.first_time = 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
