You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2023/01/12 07:13:09 UTC

[GitHub] [airflow] uranusjr commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field

uranusjr commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067772280


##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -407,3 +407,18 @@ def get_external_log_url(self, task_instance: TaskInstance, try_number: int) ->
     def supports_external_link(self) -> bool:
         """Whether we can support external links"""
         return bool(self.frontend)
+
+
+def safe_attrgetter(*items, obj, default):
+    """
+    Get items from obj but return default if not found
+
+    :meta private:
+    """
+    val = None
+    try:
+        val = attrgetter(*items)(obj)
+    except AttributeError:
+        pass
+
+    return val or default

Review Comment:
   I’d name this something like `getattr_nested`
   
   Also defauling to None does not feel like a good idea. You’ll want NOTSET. Or it’s also a good idea to just inline this logic in `_group_logs_by_host`



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org