kaxil commented on a change in pull request #8772:
URL: https://github.com/apache/airflow/pull/8772#discussion_r422529496



##########
File path: airflow/serialization/serialized_objects.py
##########
@@ -395,6 +416,60 @@ def deserialize_operator(cls, encoded_op: Dict[str, Any]) 
-> BaseOperator:
 
         return op
 
+    @classmethod
+    def _is_constructor_param(cls, attrname: str, instance: Any) -> bool:
+        # Check all super classes too
+        return any(
+            attrname in cls.__constructor_params_for_subclass(typ)
+            for typ in type(instance).mro()
+        )
+
+    @classmethod
+    def _value_is_hardcoded_default(cls, attrname: str, value: Any, instance: 
Any) -> bool:
+        """
+        Check if ``value`` is the default value for ``attrname`` as set by the
+        constructor of ``instance``, or any of it's parent classes up
+        to-and-including BaseOperator.
+
+        .. seealso::
+
+            :py:meth:`BaseSerialization._value_is_hardcoded_default`
+        """
+
+        def _is_default(ctor_params, attrname, value):
+            if attrname not in ctor_params:
+                return False
+            ctor_default = ctor_params[attrname].default
+
+            # Also returns True if the value is an empty list or empty dict.
+            # This is done to account for the case where the default value of
+            # the field is None but has the ``field = field or {}`` set.
+            return ctor_default is value or (ctor_default is None and value in 
[{}, []])
+
+        for typ in type(instance).mro():
+            ctor_params = cls.__constructor_params_for_subclass(typ)
+
+            if _is_default(ctor_params, attrname, value):
+                if typ is BaseOperator:
+                    return True
+                # For added fun, if a subclass sets a different default value 
to the
+                # same argument, (i.e. a subclass changes default of 
do_xcom_push from
+                # True to False), we then do want to include it.
+                #
+                # This is because we set defaults based on BaseOperators
+                # defaults, so if we didn't set this when inflating we'd
+                # have the wrong value
+
+                base_op_ctor_params = 
cls.__constructor_params_for_subclass(BaseOperator)
+                if attrname not in base_op_ctor_params:
+                    return True
+                return base_op_ctor_params[attrname].default == value
+
+            if typ is BaseOperator:
+                break

Review comment:
       Checking BaseOperator might be enough. I think there was a test you said 
was failing, do you know which one was that?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to