SameerMesiah97 commented on code in PR #69005:
URL: https://github.com/apache/airflow/pull/69005#discussion_r3484365213


##########
providers/edge3/src/airflow/providers/edge3/executors/edge_executor.py:
##########
@@ -340,7 +341,7 @@ def _purge_jobs(self, session: Session) -> bool:
     @provide_session
     def sync(self, *, session: Session = NEW_SESSION) -> None:
         """Sync will get called periodically by the heartbeat method."""
-        with Stats.timer("edge_executor.sync.duration"):
+        with Stats.timer("edge_executor.sync.duration", 
tags=prune_dict({"team_name": self.team_name})):

Review Comment:
   I am just wondering why this is needed if we have this already on lines 
75-76:
   
   ```
   if not hasattr(self, "team_name"):
       self.team_name = None
   ```
   
   In the older versions where BaseExecutor has no `team_name`, won't 
`self.team_name` default to `None` and this would render the `getattr(self, 
"team_name", None)` redundant? I am asking this here because I am handling it 
the exact same way as the Edge Executor does right now in the Kubernetes 
Executor i.e. default to a `None` value for  `self.team_name` at object level 
if it can not be inherited from `BaseExecutor`.



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