-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add revisionHistoryLimit to all deployments #25059
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
Add revisionHistoryLimit to all deployments #25059
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)
|
6a99947 to
71ae179
Compare
99091e7 to
fc3693a
Compare
|
Makes sense - but should not it be possible to define also "global" revisionHistoryLimit that would override all "detailed" ones if not set? That would be easier to control @jedcunningham @ephraimbuddy @dstandish ? |
|
yeah, that makes sense. I will update this PR with that global value |
38e27c4 to
5221df3
Compare
|
Done @potiuk |
potiuk
left a comment
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.
LGTM. Anyone wants to take a look @jedcunningham @dstandish @ephraimbuddy ?
5221df3 to
fa3833d
Compare
|
@csp98, can you also add corresponding sections to You'll also have to add |
31cdf40 to
0f3612e
Compare
|
Done @jedcunningham ;) |
0f3612e to
fd45fdb
Compare
4813561 to
61040b5
Compare
6953030 to
8d8fac8
Compare
potiuk
left a comment
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.
Much nicer now.
8d8fac8 to
c3ef3f3
Compare
c3ef3f3 to
d11606d
Compare
|
Hi @potiuk, @jedcunningham
What's the way to proceed? |
|
I've restarted the failures, hopefully they pass this time around. |
|
Cool @jedcunningham, thanks. |
|
No, you'll drive yourself crazy trying to keep up 👍. As long as it stays relatively up to date we are okay, plus we don't get a ton of chart changes anyways. |
Well. Actually rebasing PR is a daily routine I do. The more often you do it, the less painful it is, and the easier it is. And you learn how to rebase quickly even if you have conflicts. This might sound counter-intiuitive but it actually works. So contrary to what @jedcunningham said - yeah, by all means, rebase early, rebase often. |
|
😛 |
|
Awesome work, congrats on your first merged pull request! |
|
Daily, sure, but it was already rebased 3 hours ago 🙃 Thanks @csp98! Congrats on your first commit 🎉 |
|
Thank you all @potiuk @jedcunningham |
|
Right. I sometimes rebase few times an hour (but only when I fix something) rebasing without change is indeed not needed (mostly). Unless there is an important change in main that is. |
closes: #25058
We can use this parameter to control how many old ReplicaSets we want to retain for each deployment.
^ 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.rstor{issue_number}.significant.rst, in newsfragments.