topherinternational commented on code in PR #53821:
URL: https://github.com/apache/airflow/pull/53821#discussion_r2314509034


##########
providers/elasticsearch/tests/unit/elasticsearch/log/test_es_task_handler.py:
##########
@@ -131,7 +136,7 @@ def teardown_method(self):
     def test_es_response(self):
         sample_response = self.es.sample_log_response()
         es_response = ElasticSearchResponse(self.es_task_handler, 
sample_response)
-        logs_by_host = self.es_task_handler._group_logs_by_host(es_response)
+        logs_by_host = self.es_task_handler.io._group_logs_by_host(es_response)
 
         def concat_logs(lines):
             log_range = -1 if lines[-1].message == 
self.es_task_handler.end_of_log_mark else None

Review Comment:
   `lines[-1].message` looks not compliant with the Airflow 3 change from 
`message` to `event` - as per my comment above, this test probably falsely 
passes bc the test hits contain both `message` and `event` fields (~in the 
field, it should fail to read a hit with only the `event` field~ in the test, 
it should fail to read an Airflow 3 log hit that only has the `event` field).
   
   Also any reason we can't use the actual `concat_logs` from `es_task_handler`?
   



-- 
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]

Reply via email to