dstandish commented on code in PR #44456:
URL: https://github.com/apache/airflow/pull/44456#discussion_r1876176487
##########
airflow/dag_processing/collection.py:
##########
@@ -435,14 +456,20 @@ def add_asset_trigger_references(
refs_to_add: dict[tuple[str, str], set[str]] = {}
refs_to_remove: dict[tuple[str, str], set[str]] = {}
triggers: dict[str, BaseTrigger] = {}
+
+ # Optimization: if no asset collected, skip fetching active assets
+ active_assets = _find_active_assets(self.assets.keys(),
session=session) if self.assets else {}
+
for name_uri, asset in self.assets.items():
- asset_model = assets[name_uri]
+ # If the asset belong to a DAG not active or paused, consider
there is no watcher associated to it
+ asset_watchers = asset.watchers if name_uri in active_assets else
[]
trigger_repr_to_trigger_dict: dict[str, BaseTrigger] = {
- repr(trigger): trigger for trigger in asset.watchers
+ repr(trigger): trigger for trigger in asset_watchers
Review Comment:
I think the only thing a repr should be used for is just like if the object
is printed out or something -- it should never be used for comparisons and
stuff -- that's what `__eq__` etc are for. It should not have material code
impact, i.e. something that your logic depends on.
##########
airflow/dag_processing/collection.py:
##########
@@ -435,14 +456,20 @@ def add_asset_trigger_references(
refs_to_add: dict[tuple[str, str], set[str]] = {}
refs_to_remove: dict[tuple[str, str], set[str]] = {}
triggers: dict[str, BaseTrigger] = {}
+
+ # Optimization: if no asset collected, skip fetching active assets
+ active_assets = _find_active_assets(self.assets.keys(),
session=session) if self.assets else {}
+
for name_uri, asset in self.assets.items():
- asset_model = assets[name_uri]
+ # If the asset belong to a DAG not active or paused, consider
there is no watcher associated to it
+ asset_watchers = asset.watchers if name_uri in active_assets else
[]
trigger_repr_to_trigger_dict: dict[str, BaseTrigger] = {
- repr(trigger): trigger for trigger in asset.watchers
+ repr(trigger): trigger for trigger in asset_watchers
Review Comment:
I think the only thing a repr should be used for is just like if the object
is printed out or something -- it should never be used for comparisons and
stuff -- that's what `__eq__` etc are for. It should not have material code
impact, i.e. something that your operative logic depends on.
--
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
