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:09:14 UTC
[GitHub] [airflow] dstandish opened a new pull request, #28878: Allow nested attr in elasticsearch host_field
dstandish opened a new pull request, #28878:
URL: https://github.com/apache/airflow/pull/28878
Sometimes we may need to use nested field e.g. with filebeat:
AIRFLOW__ELASTICSEARCH__HOST_FIELD=host.name
Currently this will not fail but will return "default_host" -- the default value.
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067792774
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -409,16 +409,22 @@ def supports_external_link(self) -> bool:
return bool(self.frontend)
-def safe_attrgetter(*items, obj, default):
+def getattr_nested(obj, item, default):
"""
- Get items from obj but return default if not found
+ Get item from obj but return default if not found
+
+ E.g. calling ``getattr_nested('b.c', a, "NA")`` will return
+ ``a.b.c`` if such a value exists
:meta private:
"""
- val = None
+ NOTSET = object()
+ val = NOTSET
try:
- val = attrgetter(*items)(obj)
+ val = attrgetter(item)(obj)
except AttributeError:
pass
Review Comment:
yes, now that we're no longer being as "helpful" thanks... updated
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067776349
##########
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:
or that's the intention lemme confirm
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067776939
##########
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:
yup! lmkyt
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067789878
##########
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:
slash updated
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067782575
##########
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:
ok renamed
--
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
[GitHub] [airflow] uranusjr commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [airflow] uranusjr commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067790617
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -409,16 +409,22 @@ def supports_external_link(self) -> bool:
return bool(self.frontend)
-def safe_attrgetter(*items, obj, default):
+def getattr_nested(obj, item, default):
"""
- Get items from obj but return default if not found
+ Get item from obj but return default if not found
+
+ E.g. calling ``getattr_nested('b.c', a, "NA")`` will return
+ ``a.b.c`` if such a value exists
:meta private:
"""
- val = None
+ NOTSET = object()
+ val = NOTSET
try:
- val = attrgetter(*items)(obj)
+ val = attrgetter(item)(obj)
except AttributeError:
pass
Review Comment:
```python
try:
return attrgetter(item)(obj)
except AttributeError:
return default
```
I think this would work…?
--
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
[GitHub] [airflow] uranusjr commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067778960
##########
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:
Sorry, I meant the `val or default` part, if the attrgetter gets a None, the function would return `default` instead of `None`, inconsistent to `getattr`.
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #28878:
URL: https://github.com/apache/airflow/pull/28878#discussion_r1067776188
##########
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:
it does not default to None -- it defaults to _default_, which is required!
--
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
[GitHub] [airflow] dstandish merged pull request #28878: Allow nested attr in elasticsearch host_field
Posted by GitBox <gi...@apache.org>.
dstandish merged PR #28878:
URL: https://github.com/apache/airflow/pull/28878
--
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