-
Notifications
You must be signed in to change notification settings - Fork 17.4k
Multi Team: Automatically create and assign team default pools #69768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cfc5e88
6f60e75
1a9a37c
1ca0fb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add automatic creation of team default pools in multi-team deployments and assign tasks without an explicitly configured pool to their team's default pool during DAG parsing. Existing multi-team deployments should run ``airflow teams sync`` after upgrading to provision default pools for existing teams. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ | |
| ) | ||
| from airflow.executors.executor_loader import ExecutorLoader | ||
| from airflow.listeners.listener import get_listener_manager | ||
| from airflow.models.pool import Pool | ||
| from airflow.serialization.definitions.notset import NOTSET, ArgNotSet, is_arg_set | ||
| from airflow.serialization.serialized_objects import LazyDeserializedDAG | ||
| from airflow.utils.file import correct_maybe_zipped | ||
|
|
@@ -161,6 +162,30 @@ def _validate_executor_fields(dag: DAG, bundle_name: str | None = None) -> None: | |
| ) | ||
|
|
||
|
|
||
| def _assign_default_team_pools( | ||
| dag: DAG, | ||
| bundle_name: str | None = None, | ||
| ) -> None: | ||
| """Assign the default team pool to tasks that do not explicitly specify a pool.""" | ||
| dag_team_name = None | ||
|
|
||
| if conf.getboolean("core", "multi_team"): | ||
| if bundle_name: | ||
| from airflow.dag_processing.bundles.manager import DagBundlesManager | ||
|
|
||
| bundle_manager = DagBundlesManager() | ||
| bundle_config = bundle_manager._bundle_config[bundle_name] | ||
|
|
||
| dag_team_name = bundle_config.team_name | ||
|
|
||
| if not dag_team_name: | ||
| return | ||
|
Comment on lines
+172
to
+182
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can any of this logic be extracted from the similar steps done in _validate_executor_fields()?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered extracting a helper, but it would end up being 10 lines at most and it would only be used in 2 places. Also, I felt keeping the logic inline made each function a little easier to follow. That being said, I'm happy to extract it if you think we'll be reusing it elsewhere but I dont see the need for it right now. |
||
|
|
||
| for task in dag.tasks: | ||
| if task.pool == Pool.DEFAULT_POOL_NAME: | ||
| task.pool = Pool.get_default_team_pool_name(dag_team_name) | ||
|
SameerMesiah97 marked this conversation as resolved.
|
||
|
|
||
|
|
||
| class DagBag(LoggingMixin): | ||
| """ | ||
| A dagbag is a collection of dags, parsed out of a folder tree and has high level configuration settings. | ||
|
|
@@ -344,6 +369,7 @@ def process_file(self, filepath, only_if_updated=True, safe_mode=True): | |
| # Validate before adding to bag (matches original _process_modules behavior) | ||
| dag.validate() | ||
| _validate_executor_fields(dag, self.bundle_name) | ||
| _assign_default_team_pools(dag, self.bundle_name) | ||
| self.bag_dag(dag=dag) | ||
| bagged_dags.append(dag) | ||
| except AirflowClusterPolicySkipDag: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need the flush here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This was added because without it a foreign key violation was occuring as
Pool.create_or_update_pool()creates a pool referencingteam_name. Since the Team was just added in the same transaction, it needs to be flushed before that foreign key can be resolved.