jscheffl commented on code in PR #61646:
URL: https://github.com/apache/airflow/pull/61646#discussion_r2843241583
##########
providers/edge3/src/airflow/providers/edge3/cli/api_client.py:
##########
@@ -163,14 +169,19 @@ async def worker_set_state(
return WorkerSetStateReturn(**result)
-async def jobs_fetch(hostname: str, queues: list[str] | None,
free_concurrency: int) -> EdgeJobFetched | None:
+async def jobs_fetch(
+ hostname: str,
+ queues: list[str] | None,
+ free_concurrency: int,
+ team_name: str | None = None,
Review Comment:
Mhm, I am thinking a bit about security. So we can enable multi-team of
course but at the moment the authentication of the worker is limited to _one_
shared secret for all teams. So while technically this is a separation in terms
of security this is kind of void as the user can just switch the team name in
deployment.
This would need to be added to docs as restriction.
##########
providers/edge3/tests/unit/edge3/models/test_db.py:
##########
@@ -205,11 +204,13 @@ def test_initdb_existing_db(self, mock_get_rev,
mock_create, mock_upgrade, sessi
mock_create.assert_not_called()
def test_revision_heads_map_populated(self):
- """Test that _REVISION_HEADS_MAP is populated with the initial
migration."""
+ """Test that _REVISION_HEADS_MAP is populated with all migrations."""
from airflow.providers.edge3.models.db import _REVISION_HEADS_MAP
assert "3.0.0" in _REVISION_HEADS_MAP
assert _REVISION_HEADS_MAP["3.0.0"] == "9d34dfc2de06"
+ assert "3.0.2" in _REVISION_HEADS_MAP
+ assert _REVISION_HEADS_MAP["3.0.2"] == "a09c3ee8e1d3"
Review Comment:
3.1.0 here as well....
##########
providers/edge3/src/airflow/providers/edge3/models/edge_worker.py:
##########
@@ -357,12 +392,17 @@ def request_shutdown(worker_name: str, session: Session =
NEW_SESSION) -> None:
@provide_session
-def add_worker_queues(worker_name: str, queues: list[str], session: Session =
NEW_SESSION) -> None:
+def add_worker_queues(
+ worker_name: str, team_name: str | None, queues: list[str], session:
Session = NEW_SESSION
+) -> None:
"""Add queues to an edge worker."""
- query = select(EdgeWorkerModel).where(EdgeWorkerModel.worker_name ==
worker_name)
+ query = get_query_filter_by_team_and_worker_name(worker_name, team_name)
Review Comment:
I was thinking a moment... knowing that team name is an optional column the
worker name is still the primary key. So adding team name to parameters and
query just "ensures" that a worker is affected if correct team is provided. But
actually team is not needed as the worker name still needs to be unique.
##########
providers/edge3/src/airflow/providers/edge3/models/db.py:
##########
@@ -31,13 +30,28 @@
_REVISION_HEADS_MAP: dict[str, str] = {
"3.0.0": "9d34dfc2de06",
+ "3.0.2": "a09c3ee8e1d3",
Review Comment:
I assume multi-team is a feature, so this will then render version 3.1.0
(not 3.0.2) - unfortunately you need to make it "forward 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]
