fix(appset): scmProvider generator filters not applying properly (#14420) #14465
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14465 +/- ##
==========================================
- Coverage 49.53% 49.52% -0.01%
==========================================
Files 269 269
Lines 47222 47208 -14
==========================================
- Hits 23392 23381 -11
+ Misses 21530 21528 -2
+ Partials 2300 2299 -1 ☔ View full report in Codecov by Sentry. |
crenshaw-dev
changed the title
crenshaw-dev
changed the title
|
This PR is a few months old. What is the next step to getting review and approval from the Argo team? |
|
@cyrus-mc the prerequisites are done, it's down to review time at this point. Contributions currently exceed review capacity. |
|
@crenshaw-dev bump 🙏 |
|
bump 🙏🏽 |
|
Please review this one. We are having the same problem, which right now makes multi-repo architecture with discovery very difficult :( Thank you! |
|
Would really love to see this fix get in! |
|
Good morning everyone, It's also an urgent fix for us ! Thanks community |
Signed-off-by: Jedrzej Kotkowski <[email protected]>
| // FilterTypeRepo are filters that apply to the repo itself (name, labels) | ||
| FilterTypeRepo bool | ||
| // FilterTypeBranch are filters that apply to the repo content (path, branch) | ||
| FilterTypeBranch bool | ||
| } | ||
|
|
||
| // A convenience type for indicating where to apply a filter | ||
| type FilterType int64 | ||
|
|
||
| // The enum of filter types | ||
| const ( | ||
| FilterTypeUndefined FilterType = iota | ||
| FilterTypeBranch | ||
| FilterTypeRepo | ||
| ) |
There was a problem hiding this comment.
Not sure if it's a good idea to use bools instead of just using the existing typed field.
What is the reasoning for this refactor?
There was a problem hiding this comment.
FilterType would allow the filter to be of only a single type, but in reality it can be of both types - hence splitting it into two bools
|
bump |
|
Any news on this one? It is killing GitHub repository auto-discovery every hour because of API rate limit :( |
|
Without this fix |
|
Any update on this PR? |
|
Bump: this pr fixes a critical issues. |
|
If you are following this thread the newest version of Argo release, had a similar fix and we have seen our request go down. |
Where? I can’t see references to this problem on the latest releases. |
|
I didn't see any reference to improving this either, but I did update from 2.12.3 to 2.13.0-rc1 and we do see the rate improve quite a bit. This is our internal tracking of our github API quota: It's still a lot and if you're not very careful will eat up your quota. (We have a hack in place where we keep the applicationset controller down to 0 replicas 80% of the time.) |
It’s in the release notes for 1568165 on 12.2.4 |
Great, thank you very much! As this fix is present in 2.12.4, let's try it :) |
Fixes #14420
This PR fixes multiple scmProvider filters not applying properly when using combinations of such as repositoryMatch + branchMatch or multiple filters, as described in the mentioned issue.
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.