hterik commented on code in PR #29929:
URL: https://github.com/apache/airflow/pull/29929#discussion_r1135066602


##########
airflow/kubernetes/kubernetes_helper_functions.py:
##########
@@ -119,3 +120,7 @@ def annotations_to_key(annotations: dict[str, str]) -> 
TaskInstanceKey:
         try_number=try_number,
         map_index=map_index,
     )
+
+
+def annotations_to_str(annotations: dict[str, str]) -> str:
+    return json.dumps(annotations)

Review Comment:
   Having all info in one log entry makes it easier to do things like grep for 
all entries with dag_id=xxxx.  Otherwise you need to keep context in mind and 
remember what was logged above and trust that such lines belong together.
   
   The events logged in `process_status` don't have the TI logged together nor 
on any adjacent lines before.
   Events like the following appear outof nowehere in the log today, 
`{kubernetes_executor.py:213} INFO - Event: 
dagfoo-taskbar-2aa04e01a1a945cfb1c59750ff7f4f70 Succeeded`
   You have to scroll down far in the log to reach `Changing state of ... pod 
name`, or very very far up the log to reach `Creating kubernetes pod ... with 
pod name` to associate them
   
   It will get rather wordy, but imo the verbosity of having few entries with a 
lot of information is better than verbosity of multiple lines with spread out 
information.
   
   
   -------------
   
   execution_date is redundant when run_id is included right? 
   



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