ashb commented on code in PR #53870:
URL: https://github.com/apache/airflow/pull/53870#discussion_r2253765726
##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -529,6 +529,18 @@ def to_dict(self, *, prune_empty: bool = False, validate:
bool = True) -> dict[s
:meta private:
"""
+ if hasattr(sys.modules.get("airflow.sdk.execution_time.task_runner"),
"SUPERVISOR_COMMS"):
Review Comment:
Do we need this on to_dict ? It's an instance method, and I'm not sure you
end up with an object of this class in SDK context?
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -30,6 +30,49 @@
log = logging.getLogger(__name__)
+def _prune_dict(val: Any, mode="strict"):
+ """
+ Given dict ``val``, returns new dict based on ``val`` with all empty
elements removed.
+
+ What constitutes "empty" is controlled by the ``mode`` parameter. If mode
is 'strict'
+ then only ``None`` elements will be removed. If mode is ``truthy``, then
element ``x``
+ will be removed if ``bool(x) is False``.
+ """
+
+ def is_empty(x):
+ if mode == "strict":
+ return x is None
+ if mode == "truthy":
+ return bool(x) is False
+ raise ValueError("allowable values for `mode` include 'truthy' and
'strict'")
Review Comment:
However I only ever see this called with "strict"?
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -30,6 +30,49 @@
log = logging.getLogger(__name__)
+def _prune_dict(val: Any, mode="strict"):
+ """
+ Given dict ``val``, returns new dict based on ``val`` with all empty
elements removed.
+
+ What constitutes "empty" is controlled by the ``mode`` parameter. If mode
is 'strict'
+ then only ``None`` elements will be removed. If mode is ``truthy``, then
element ``x``
+ will be removed if ``bool(x) is False``.
+ """
+
+ def is_empty(x):
+ if mode == "strict":
+ return x is None
+ if mode == "truthy":
+ return bool(x) is False
+ raise ValueError("allowable values for `mode` include 'truthy' and
'strict'")
Review Comment:
```suggestion
if mode == "strict":
is_empty = lambda x: x is None
if mode == "truthy":
is_empty = lambda x: bool(x) is False
else:
raise ValueError("allowable values for `mode` include 'truthy' and
'strict'")
```
##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -567,6 +591,18 @@ def from_json(cls, value, conn_id=None) -> Connection:
def as_json(self) -> str:
"""Convert Connection to JSON-string object."""
+ if hasattr(sys.modules.get("airflow.sdk.execution_time.task_runner"),
"SUPERVISOR_COMMS"):
Review Comment:
Ditto -- I think the only compat we need is the classmethod
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -160,6 +203,14 @@ def get(cls, conn_id: str) -> Any:
@property
def extra_dejson(self) -> dict:
"""Deserialize `extra` property to JSON."""
+ return self.get_extra_dejson()
+
+ def get_extra_dejson(self, nested: bool = False) -> dict:
Review Comment:
Nit: Do it the other way around please so we can deprecate the method someday
--
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]