cBiscuitSurprise commented on code in PR #34747:
URL: https://github.com/apache/airflow/pull/34747#discussion_r1346370418


##########
airflow/providers/amazon/aws/log/cloudwatch_task_handler.py:
##########
@@ -33,6 +33,24 @@
     from airflow.models import TaskInstance
 
 
+def json_serialize_legacy(o):
+    """Json serializer replicating legacy watchtower behavior.
+
+    The legacy `[email protected]` json serializer function that serialized
+    datetime objects as ISO format andall other non-JSON-serializable to 
`null`.
+    """
+    if isinstance(o, (date, datetime)):
+        return o.isoformat()
+
+
+"""json serializer replicating current watchtower behavior

Review Comment:
   fixed



##########
airflow/providers/amazon/aws/log/cloudwatch_task_handler.py:
##########
@@ -33,6 +33,24 @@
     from airflow.models import TaskInstance
 
 
+def json_serialize_legacy(o):
+    """Json serializer replicating legacy watchtower behavior.
+
+    The legacy `[email protected]` json serializer function that serialized
+    datetime objects as ISO format andall other non-JSON-serializable to 
`null`.

Review Comment:
   fixed



##########
airflow/providers/amazon/aws/log/cloudwatch_task_handler.py:
##########
@@ -33,6 +33,24 @@
     from airflow.models import TaskInstance
 
 
+def json_serialize_legacy(o):
+    """Json serializer replicating legacy watchtower behavior.

Review Comment:
   fixed



##########
airflow/config_templates/config.yml:
##########
@@ -928,6 +928,21 @@ logging:
       type: boolean
       example: ~
       default: "False"
+    json_serializer:
+      description: |
+        By default, for non-string logged messages all non-json-parsable 
objects are logged as `null` except
+        `datetime` objects which are ISO formatted. Users can optionally 
provide their own JSON serializer or
+        opt to use a `repr` serializer which calls `repr(object)` for any 
non-JSON-serializable objects in the
+        logged message. The 
`airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize` uses
+        `repr` while 
`airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize_legacy`
 uses
+        `null`. If a custom serializer is provide, it must adhear to 
`Callable[[Any], str]`
+        (`def my_serializer(o: Any) -> str`). Be aware, that if opting in to 
using the `repr` serializer, you
+        should take extra care that no new, sensitive, data is logged (e.g. 
credentials). If creating your own
+        json-serializer take special care to fail gracefully, without throwing.

Review Comment:
   updated to 
   
   ```python
   """
   ... Since this is on the logging path, special care should be taken
   to fail gracefully, without raising a new exception as it's possible an 
exception is already being
   handled (and logged)....
   """
   ```



##########
airflow/config_templates/config.yml:
##########
@@ -928,6 +928,21 @@ logging:
       type: boolean
       example: ~
       default: "False"
+    json_serializer:
+      description: |
+        By default, for non-string logged messages all non-json-parsable 
objects are logged as `null` except
+        `datetime` objects which are ISO formatted. Users can optionally 
provide their own JSON serializer or
+        opt to use a `repr` serializer which calls `repr(object)` for any 
non-JSON-serializable objects in the
+        logged message. The 
`airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize` uses
+        `repr` while 
`airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize_legacy`
 uses
+        `null`. If a custom serializer is provide, it must adhear to 
`Callable[[Any], str]`
+        (`def my_serializer(o: Any) -> str`). Be aware, that if opting in to 
using the `repr` serializer, you
+        should take extra care that no new, sensitive, data is logged (e.g. 
credentials). If creating your own

Review Comment:
   fixed, I simplified this blob of text a little



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