-
Notifications
You must be signed in to change notification settings - Fork 9
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 conditional read after write for rules #584
base: main
Are you sure you want to change the base?
Conversation
NOTE: Rebased from main after refactor in #13014 Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
…ch request Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Danny Kopping <[email protected]>
Updated & added tests Signed-off-by: Danny Kopping <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
for i, rule := range g.rules { | ||
select { | ||
case <-g.done: | ||
return | ||
default: | ||
} | ||
|
||
func(i int, rule Rule) { | ||
eval := func(i int, rule Rule, independent, async bool) { |
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.
instead of propagating this all the way way down and modifying a couple of interfaces, can't we just move NewOriginContext(ctx, NewRuleDetail(rule, independent))
from the rule to g.Eval
(i.e. this function)?
// Independent holds whether this rule depends on the result of other rules | ||
// within the same rule group or not. |
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.
this forgets to mention that if other rules depend on this rule Independent
is also true
Danny Kopping seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
In this PR:
RuleDetail.Independent
RuleConcurrencyController
intoRuleConcurrencyController
andRuleDependencyController