Skip to content

Conversation

@o-nikolas
Copy link
Contributor

@o-nikolas o-nikolas commented Dec 6, 2024

When multi team (AIP-67) is eventually in place, the scheduler will need to initialize a set of executors for each team. This change is a first step towards that which updates the ExecutorName data model and the executor loader to be "team aware".


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added the area:Executors-core LocalExecutor & SequentialExecutor label Dec 6, 2024
When multi team is eventually in place, the scheduler will need to
initialize a set of executors for each team. This change is a first
step towards that which updates the ExecutorName data model and the
executor loader to be "team aware".
@o-nikolas o-nikolas force-pushed the multi_team/onikolas/multi_executors_loader branch from 250f26c to 30396d8 Compare December 6, 2024 02:38
@o-nikolas
Copy link
Contributor Author

CC @potiuk

Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that it is done as part of AIP-67 - I think that it's worth mentioning it in the title and contents, as well as creating a project for that so it will become easier to track.
Before merging, it would be nice to outline a roadmap (general guidelines) so it would be clearer to understand the broader context of this task :)

@o-nikolas
Copy link
Contributor Author

I assume that it is done as part of AIP-67 - I think that it's worth mentioning it in the title and contents, as well as creating a project for that so it will become easier to track.

Yupp this is AIP-67. That's a fair call out, I'll update the description, title and add a tag.
I have a project plan document here which you can read if you're interested. I planned to also create a project board but am still working through the process of disambiguation and really feeling out how it will be pulled off. Once I have a few of the more foundational topics sorted and I have more clarity I will put a board together.

Before merging this one, it would be nice to have a roadmap so it would be clearer to understand the broaded context of this task 🙂

As I said above, I'll work on this, but it has never before been a requirement of merging PRs. And I don't think it's wise to add that barrier to our contribution process. So I don't think it should block this PR's merge.

@o-nikolas o-nikolas changed the title Introduce team id to executor names and loader AIP-67 - Introduce team id to executor names and loader Dec 13, 2024
@o-nikolas o-nikolas merged commit 957bd24 into apache:main Dec 13, 2024
50 checks passed
@o-nikolas o-nikolas deleted the multi_team/onikolas/multi_executors_loader branch December 13, 2024 22:58
jedcunningham added a commit to astronomer/airflow that referenced this pull request Dec 13, 2024
That import was removed in apache#44839, but apache#44710 wasn't up-to-date with main so
static checks there didn't fail. This simply adds it back.
jedcunningham added a commit that referenced this pull request Dec 14, 2024
That import was removed in #44839, but #44710 wasn't up-to-date with main so
static checks there didn't fail. This simply adds it back.
@shahar1
Copy link
Contributor

shahar1 commented Dec 14, 2024

I assume that it is done as part of AIP-67 - I think that it's worth mentioning it in the title and contents, as well as creating a project for that so it will become easier to track.

Yupp this is AIP-67. That's a fair call out, I'll update the description, title and add a tag. I have a project plan document here which you can read if you're interested. I planned to also create a project board but am still working through the process of disambiguation and really feeling out how it will be pulled off. Once I have a few of the more foundational topics sorted and I have more clarity I will put a board together.

Before merging this one, it would be nice to have a roadmap so it would be clearer to understand the broaded context of this task 🙂

As I said above, I'll work on this, but it has never before been a requirement of merging PRs. And I don't think it's wise to add that barrier to our contribution process. So I don't think it should block this PR's merge.

I apologize if it was interpreted as a barrier/blocker - it wasn't my intention at all. The doc that you created is exactly what I was looking for, just to understand how it's going to work.

got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
When multi team (AIP-67) is eventually in place, the scheduler will need to initialize a set of executors for each team. This change is a first step towards that which updates the ExecutorName data model and the executor loader to be "team aware"
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
That import was removed in apache#44839, but apache#44710 wasn't up-to-date with main so
static checks there didn't fail. This simply adds it back.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Executors-core LocalExecutor & SequentialExecutor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants