-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
18400 chart custom pod annotations #18481
Conversation
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
ed855a5
to
82969b5
Compare
b9041bd
to
75c04f6
Compare
75c04f6
to
6b9861e
Compare
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
Giving CI another shot... |
Thanks for the review. What's up with the pipeline? Seems like flakiness to me, and the helm test failure happens to your 1.20 docs-only change as well. |
Yeah sorry, it's been particularly painful the last few days unfortunately. I'll try again now, and I can run the full suite locally if that still doesn't do it. |
Yeah. I hope (🤞 ) We finally fixed all the "major" flakiness problems - so the last error here is the real failing test :) |
Close/reopen to rebuild. the Helm Chart might however still hang (and that might be a general problem) - let's see |
The helm chart unit test passes! (I do think it's general flakiness rather than real test failures.. don't know enough about the CI tests to know why, maybe because of the parallel execution.) |
Are we ok with merging this, given that the temperamental test is luckily in a passing state? 😆 |
Well. I would not say lucky. We are really just about to reach the state of "back to green" on main. I think there is one very intermittent issue currently that I know about :D |
Awesome work, congrats on your first merged pull request! |
closes: #18400
The change allows adding different custom pod annotations to the different airflow components (whereas the current chart can only add the same set of custom pod annotations to all airflow components).
Some notes:
.Values.airflowPodAnnotations
airflowPodAnnotations
, which excludesredis
etc. I don't know why we haveairflowPodAnnotations
in some components but not in others, but I want to just continue to convention to keep things clean.triggerer
as well.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.