Skip to content
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 a rate_applies_when field to Policy Rules #688

Merged
merged 3 commits into from
Sep 23, 2021

Conversation

jiffyclub
Copy link
Contributor

@jiffyclub jiffyclub commented Sep 2, 2021

Explain pull request

This is implementing my proposal from #666. The rate_applies_when field allows people constructing policy rules with attached rates to specify when the rate is applicable: is it when something is within the rule bounds or when it is outside of them? The rate_applies_when when field may take on the values in_bounds and out_of_bounds. It is used in conjunction with the rule minimum and maximum fields. It defaults to out_of_bounds.

I also used this PR to bring the Policy JSON schema and an example policy up-to-date following the merger of #658.

Is this a breaking change

  • I'm not sure

This field is optional so it doesn't break in that sense, but having a defined notion of when rates apply could change the interpretation of existing policies.

Impacted Spec

  • policy

Additional context

The MDS policy spec does not currently specify when rates apply. In #658 @avatarneil argued that they apply when an event falls outside the rule bounds, but in the rate rule examples in the example policies the rates are applying to events/counts within the rule bounds. With the merger of #658 we now have example rules that are not consistent in their interpretation of when rates apply. (The example rate policies do not specify minimum or maximum fields, so the are implicitly 0 and infinity, respectively, though perhaps it could be argued that minimum and maximum are not applicable to rate type rules.)

See also my proposal in #666.

The rate_applies_when field allows people constructing policy
rules with attached rates to specify when the rate is applicable:
is when something is within the rule bounds or when it is outside
of them? The rate_applies_when when field may take on the values
in_bounds and out_of_bounds.
@jiffyclub jiffyclub requested a review from a team as a code owner September 2, 2021 18:06
@schnuerle schnuerle added the Policy Specific to the Policy API label Sep 2, 2021
@schnuerle schnuerle added this to the 1.2.0 milestone Sep 2, 2021
@schnuerle schnuerle linked an issue Sep 2, 2021 that may be closed by this pull request
@schnuerle
Copy link
Member

Per our working group meeting could you add some example updates to this PR to show how minimum and maximum would work?

@schnuerle
Copy link
Member

Another note from the meeting, when updating the schema, remember to update the files in the schema templates folder too. That's what's used to generate the schema files.

As a bonus also includes an inclusive_maximum example.
@jiffyclub
Copy link
Contributor Author

I've added an example in d49ae26 and updated the schema template in f38cc96.

One question I had was whether I needed to add rate_applies_when: "in_bounds" to the example rate policies. I can't really tell if minimum/maximum have meaning for those, but if they do they would need rate_applies_when: "in_bounds" since the range is 0 - infinity. Or maybe rate_applies_when doesn't apply when there's no bounds on the rule, the rate always applies.

@schnuerle
Copy link
Member

I've added an example in d49ae26 and updated the schema template in f38cc96.

One question I had was whether I needed to add rate_applies_when: "in_bounds" to the example rate policies. I can't really tell if minimum/maximum have meaning for those, but if they do they would need rate_applies_when: "in_bounds" since the range is 0 - infinity. Or maybe rate_applies_when doesn't apply when there's no bounds on the rule, the rate always applies.

@avatarneil @marie-x could you weigh in on this question so we can finish this PR for the release?

@avatarneil
Copy link
Contributor

I don't think we need to add it. The new (as of 1.2) rate policy examples use the default out_of_bounds methodology, and I think for the existing rate policies (as Matt mentioned) rate_applies_when doesn't apply when there are no bounds on the rule.

@jiffyclub
Copy link
Contributor Author

I would argue that if anything the existing rate examples are using in_bounds, since the bounds are infinite if unspecified. But I do think it's fair to say that if min and max are not specified a rate is clearly meant to apply in all cases.

@schnuerle
Copy link
Member

schnuerle commented Sep 20, 2021

Looks ready to approve. I'm a little concerned that removing 'once' as an option completely, replacing it with 'once_on_unmatch' and 'once_on_match' may break some people's implementations if they are looking at 'once', but it would more likely just not have their code find these new options, which I think is ok as it's not breaking their processing. This change will be in the release notes and schema for 1.2 which should be adequate.

@schnuerle schnuerle merged commit 5b96ea8 into openmobilityfoundation:dev Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Policy Specific to the Policy API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a rate_applies_when field to policy rules
3 participants