potiuk commented on code in PR #40589:
URL: https://github.com/apache/airflow/pull/40589#discussion_r1669002851
##########
airflow/providers/openlineage/conf.py:
##########
@@ -145,3 +145,8 @@ def execution_timeout() -> int:
"""[openlineage] execution_timeout."""
option = conf.get(_CONFIG_SECTION, "execution_timeout", fallback="")
return _safe_int_convert(str(option).strip(), default=10)
+
+
+@cache
+def include_full_task_info() -> bool:
+ return _is_true(conf.get(_CONFIG_SECTION, "include_full_task_info",
fallback="False"))
Review Comment:
Well. It makes a bit inconsistent behaviour with other boolean flags, which
might be surprising for the users. And if there is no good reason to diverge
from the common approach, I think consistency trumps convenience (if you think
the approach you proposed is a bit more convenient in some cases):
Is there a good reason why the common `getboolean` behaviour is a problem
and can't be used ?
I mean this:
```python
def getboolean(self, section: str, key: str, **kwargs) -> bool: # type:
ignore[override]
val = str(self.get(section, key, _extra_stacklevel=1,
**kwargs)).lower().strip()
if "#" in val:
val = val.split("#")[0].strip()
if val in ("t", "true", "1"):
return True
elif val in ("f", "false", "0"):
return False
else:
raise AirflowConfigException(
f'Failed to convert value to bool. Please check "{key}" key
in "{section}" section. '
f'Current value: "{val}".'
)
```
--
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]
