-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix(appset): add ApplicationSet ProgressiveSync handling to clean up old appStatus entries when Applications are removed or RollingSync is disabled #13419
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #13419 +/- ##
==========================================
+ Coverage 49.20% 49.21% +0.01%
==========================================
Files 248 248
Lines 42909 42839 -70
==========================================
- Hits 21113 21084 -29
+ Misses 19682 19656 -26
+ Partials 2114 2099 -15
☔ View full report in Codecov by Sentry. |
@wmgroot how far back should we cherry-pick this? |
This bug is present in the initial release of Progressive Syncs, so it could be cherrypicked to the next patch release of 2.6. Do you think that's necessary for an alpha feature though? |
As long as it's safe, and the cherry pick isn't gnarly, I'm in favor of cherry-picking. |
That sounds fine to me, I think the change should be clean. What's the process for a cherry-pick? Should I open a second PR against the |
/cherry-pick release-2.6 |
/cherry-pick release-2.7 |
We'll see how the bot copes 😄 |
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.
I was able to test the PR with ProgressiveSyncs and was able to see the application statuses getting cleaned up.
I also went through the code and didn't spot anything weird. Just a minor nit.
…ntries when Applications are removed or RollingSync is disabled Signed-off-by: wmgroot <wmgroot@gmail.com>
3efed78
to
5a167d1
Compare
Updated and rebased. |
Cherry-pick failed with |
… old appStatus entries when Applications are removed or RollingSync is disabled (#13419) Signed-off-by: wmgroot <wmgroot@gmail.com>
… old appStatus entries when Applications are removed or RollingSync is disabled (argoproj#13419) (argoproj#13759) Signed-off-by: wmgroot <wmgroot@gmail.com> Co-authored-by: wmgroot <wmgroot@gmail.com> Signed-off-by: schakrad <58915923+schakrad@users.noreply.github.com>
… old appStatus entries when Applications are removed or RollingSync is disabled (argoproj#13419) Signed-off-by: wmgroot <wmgroot@gmail.com>
… old appStatus entries when Applications are removed or RollingSync is disabled (argoproj#13419) Signed-off-by: wmgroot <wmgroot@gmail.com>
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
Please see Contribution FAQs if you have questions about your pull-request.
Closes #13331