potiuk commented on code in PR #39825:
URL: https://github.com/apache/airflow/pull/39825#discussion_r1671253845


##########
airflow/models/baseoperator.py:
##########
@@ -1245,11 +1245,21 @@ def dag(self) -> DAG:  # type: ignore[override]
     @dag.setter
     def dag(self, dag: DAG | None):
         """Operators can be assigned to one DAG, one time. Repeat assignments 
to that same DAG are ok."""
-        from airflow.models.dag import DAG
-
         if dag is None:
             self._dag = None
             return
+
+        # if already set to elided, then just set and exit
+        if self._dag is ELIDED_DAG:

Review Comment:
   I think VISITED or ALREADY_SERIALIZED is better name - it comes from the 
other direction - describes what happens with the dag and based on that, 
serialization code decides what to do. tthe ELIDE or MISSING is a bit "future" 
or "crystal ball" approach. How do you know you should elide it a the moment 
you add it to the kist of dags ? Turning that around and naming it as "VISITED" 
or "ALREADY_SERIALIZED" is IMHO better. The code will read as 
   
   ```
   if THIS NODE WAS VISITED ALREADY:
      skip it
   ``
   
   Rather than:
   
   ```
   if SHOULD_I_SKIP_IT:
      skip it
   ```
   
   The second case is kinda duplication and "future looking".



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

Reply via email to