topherinternational commented on code in PR #53821:
URL: https://github.com/apache/airflow/pull/53821#discussion_r2314502658
##########
providers/elasticsearch/tests/unit/elasticsearch/log/elasticmock/fake_elasticsearch.py:
##########
@@ -104,19 +104,22 @@ def sample_log_response(self, headers=None, params=None):
"file": {
"path":
"/opt/airflow/Documents/GitHub/airflow/logs/"
"dag_id=example_bash_operator'"
-
"/run_id=owen_run_run/task_id=run_after_loop/attempt=1.log"
+
"/run_id=run_run/task_id=run_after_loop/attempt=1.log"
},
"offset": 0,
},
"log.offset": 1688888863907337472,
- "log_id":
"example_bash_operator-run_after_loop-owen_run_run--1-1",
+ "log_id":
"example_bash_operator-run_after_loop-run_run--1-1",
"message": "Dependencies all met for "
"dep_context=non-requeueable deps "
"ti=<TaskInstance: "
- "example_bash_operator.run_after_loop "
- "owen_run_run [queued]>",
+ "example_bash_operator.run_after_loop ",
"task_id": "run_after_loop",
"try_number": "1",
+ "event": "Dependencies all met for "
Review Comment:
I think I see a problem here - this hit already has a `message` field, so
adding the `event` field makes it a hybrid Airflow 2/3 log instead of changing
it from Airflow 2 format to Airflow 3. IOW I don't think we'd ever encounter a
hit that looked like this in actual operation.
IMO these tests should be changed thusly:
- `sample_log_response()` should be split into major-version-specific
functions, e.g. `sample_airflow_2_log_response()` and
`sample_airflow_3_log_response()` and these `event` fields only added to the
latter.
- then the test function `test_es_response()` in
`TestElasticsearchTaskHandler` should be duplicated (or parameterized) into two
functions specific to Airflow 2 and 3 logs respectively, using the two
log-response functions in `FakeElasticsearch`.
This should test that the guts of ESTaskHandler can properly consume
ESResponses from Airflow 2 and 3.
--
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]
