vincbeck commented on code in PR #45562:
URL: https://github.com/apache/airflow/pull/45562#discussion_r1918886705


##########
airflow/triggers/base.py:
##########
@@ -124,6 +124,11 @@ def __repr__(self) -> str:
         classpath, kwargs = self.serialize()
         return self.repr(classpath, kwargs)
 
+    def __eq__(self, other: Any) -> bool:

Review Comment:
   That's for a test. Without it, the test fails, I dont remember which one 
though. I'll remove it to find out. We can see from there if it should be 
addressed that way



##########
airflow/serialization/serialized_objects.py:
##########
@@ -297,6 +306,25 @@ def decode_asset_condition(var: dict[str, Any]) -> 
BaseAsset:
     raise ValueError(f"deserialization not implemented for DAT {dat!r}")
 
 
+def decode_asset(var: dict[str, Any]) -> BaseAsset:
+    """
+    Decode a previously serialized asset.
+
+    :meta private:
+    """
+
+    def _decode_trigger(trigger_infos: dict[str, Any]) -> BaseTrigger:
+        return 
import_string(trigger_infos["classpath"])(**trigger_infos["kwargs"])

Review Comment:
   Thanks a lot for the thorough review! You are right, we do not need to 
create the trigger object. I was creating the trigger object to be consistent 
whether the source was coming from the DAG or the DB (serialized DAG). But as 
you pointed out, we can handle the 2 use cases differently and that way no need 
to create the trigger object



##########
tests/timetables/test_assets_timetable.py:
##########
@@ -142,6 +142,7 @@ def test_serialization(asset_timetable: 
AssetOrTimeSchedule, monkeypatch: Any) -
                     "uri": "test://asset/",
                     "group": "asset",
                     "extra": {},
+                    "watchers": [],

Review Comment:
   That makes sense! I removed it if there is no watcher



-- 
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-unsubscr...@airflow.apache.org

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

Reply via email to