ferruzzi commented on code in PR #69005:
URL: https://github.com/apache/airflow/pull/69005#discussion_r3484117644
##########
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:
Looks like Edge has support for 3.0+; self.team_name was added in 3.1 so
this could be a problem. This should be safer:
```suggestion
with Stats.timer("edge_executor.sync.duration",
tags=prune_dict({"team_name": getattr(self, "team_name", None)}})):
```
##########
providers/edge3/src/airflow/providers/edge3/executors/edge_executor.py:
##########
Review Comment:
It should be done, but it falls under the edge_worker metrics which he said
he'll do in another batch, which is fair. You could argue it's the same file
and may as well do it now, or you could argue to keep related changes bundled;
I'm indifferent as long as it gets done.
##########
providers/edge3/tests/unit/edge3/executors/test_edge_executor.py:
##########
@@ -120,6 +120,38 @@ def test_sync_orphaned_tasks(self, mock_stats_incr):
assert jobs[0].task_id == "started_running_orphaned"
assert jobs[0].state == TaskInstanceState.REMOVED
+ @patch(f"{Stats.__module__}.Stats.timer")
Review Comment:
This part is always tricky. I am pretty sure the right answer is `not
AIRFLOW_V_3_1_PLUS`.
Edge's airflow version support is `apache-airflow>=3.0.0,!=3.1.0` and
team_name doesn't exist on 3.0, so we should use `not AIRFLOW_V_3_2_PLUS` on
this one, but 3.1+ in the others (unless they also exclude 3.1 explicitly)
--
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]
