Skip to content

Add configurable watcher_build_coordinator_priority_weight making run build operator with priority#1989

Closed
pankajkoti wants to merge 10 commits into
mainfrom
build-priority-weight
Closed

Add configurable watcher_build_coordinator_priority_weight making run build operator with priority#1989
pankajkoti wants to merge 10 commits into
mainfrom
build-priority-weight

Conversation

@pankajkoti
Copy link
Copy Markdown
Contributor

The PR introduces [cosmos] watcher_build_coordinator_priority_weight Airflow setting (default 9999) in cosmos/settings.py.
DbtBuildCoordinatorOperator now reads this value and passes it to BaseOperator(priority_weight=…), ensuring the build-coordinator task runs before others.

closes: #1959
closes: https://github.com/astronomer/oss-integrations-private/issues/237

@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 19, 2025

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit c48cd85
🔍 Latest deploy log https://app.netlify.com/projects/sunny-pastelito-5ecb04/deploys/68cd4283fe066a00083d8b5d

@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 19, 2025

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit b71b079
🔍 Latest deploy log https://app.netlify.com/projects/sunny-pastelito-5ecb04/deploys/68d50b6c39f5cf0008ab1e04

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.78%. Comparing base (8627d63) to head (b71b079).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1989      +/-   ##
==========================================
+ Coverage   97.67%   97.78%   +0.11%     
==========================================
  Files          87       87              
  Lines        5371     5463      +92     
==========================================
+ Hits         5246     5342      +96     
+ Misses        125      121       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

Hi @pankajkoti !

Thanks for adding this.

Cosmos users are already overwhelmed with configurations. The implementation in c48cd85 gives users yet another configuration.

Let's decide on a priority for the runner task - which should be higher than the priority we set for the watcher tasks - and we can just hard-code them.

Ultimatelly, the problem we're trying to solve is to make sure the watchers do not start before the runner task - and hard-coding values would be good enough.

@pankajkoti
Copy link
Copy Markdown
Contributor Author

Hi @pankajkoti !

Thanks for adding this.

Cosmos users are already overwhelmed with configurations. The implementation in c48cd85 gives users yet another configuration.

Let's decide on a priority for the runner task - which should be higher than the priority we set for the watcher tasks - and we can just hard-code them.

Ultimatelly, the problem we're trying to solve is to make sure the watchers do not start before the runner task - and hard-coding values would be good enough.

In this case, the configuration comes with a default value, so most users won’t ever need to touch it. By contrast, if we hard-code it, we leave no room for advanced users who actively use task priorities to adapt things to their deployment setup. This knob is really aimed at that group, not the average user.

Airflow itself exposes a wide surface of configurations, and as long as we provide sensible defaults and good documentation, I don’t think the presence of this option adds friction. Users can safely ignore it unless they have a reason not to.

That said, if there’s a strong consensus that it should be hard-coded, I’m okay to remove it as a configurable parameter — I just wanted to highlight the tradeoff for those advanced cases.

@pankajkoti pankajkoti requested a review from tatiana September 22, 2025 12:48
@tatiana
Copy link
Copy Markdown
Collaborator

tatiana commented Sep 23, 2025

Hi @pankajkoti , thanks for sharing the benefits of exposing this as a configuration.
One of our following goals will be to simplify Cosmos and reduce the number of configurations, thereby improving adoption and allowing us to focus on areas that deliver the most value. We can see from support calls that several users feel lost due to the numerous configurations available.
I remain in favour of hard-coding this value for now.
Users will be able to override this value as part of #1972, without the need for a custom Airflow configuration.

@pankajkoti
Copy link
Copy Markdown
Contributor Author

Closing in favour of #1995

@pankajkoti pankajkoti closed this Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Guarantee the build task is run before sensors [ExecutionMode.WATCHER]

2 participants