* I don't think we need to make these structs for tunables in the future because we can always add a struct that adds additional options alongside AND it's likely the tunables would be different by event type.
--
You received this message because you are subscribed to the Google Groups "kubernetes-api-reviewers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-api-rev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAH16ShK6Xh%3DE%2B5WzmqEKYMLenvObBFEmxT3kHq__fkBY_qs4ZQ%40mail.gmail.com.
On Wed, May 19, 2021 at 7:44 AM Clayton Coleman <ccol...@redhat.com> wrote:TL:DR: Establishing a novel convention about naming policy that occurs "on some change" that doesn't quite fit existing use.Wanted to suggest a new convention for naming for a specific use case - opinions welcome and desired:https://github.com/kubernetes/kubernetes/pull/99378 adds a policy struct PersistentVolumeClaimPolicy to statefulsets (probably to be renamed PersistentVolumeClaimRetentionPolicy for clarity).Within the struct there are two fields for the PVC retention policy on specific events/hooks - "deletion of the set" and "scale up or down" (deletion of a pod). It's a struct so we could in theory add more things later including both numeric properties (a delay?) or more events/hooks (hypothetical "on new revision".The original choice for names was `OnSetDeleted` and `OnScaleDown`. To me that's novel (the On*) pattern. We discussed omitting the prefix `Deleted` / `ScaleDown` but since this is a policy "in a circumstance" AND there might be other fields in here that weren't "in a circumstance" it was better to preface them with something.Existing art:TopologySpreadConstraint on pod uses `WhenUnsatisfiable` - describes the policy behavior in the context of other policy fields.Pod lifecycle hooks describe the event `PostStart`, `PreStop` - a hook is always done "at something" and so "When/On" would be redundant.I don't remember / couldn't find another example of "describing policy in the context of an event" EXCEPT for the general pattern of update, which is that we prefer to describe the policy in terms of rules only, not events (maxUnavailable).Proposed:* We need to give the field a prefix because there are potentially mixed "rules" and "conditional behavior" in the future in the struct. Also, I don't think a user would see "persistentVolumeClaimRetentionPolicy.deleted" in the object and automatically understand "oh, that's *when* it's deleted" - we have help them by being explicit. (following the lifecycle hook example is not clear to the user)* I think the prefix should be "When*" instead of "On*" because we at least use When today elsewhere and it reads more naturally to me "persistentVolumeClaimRetentionPolicy.whenDeleted = Reclaim". However, this is not backed up by prior art and "On*" is slightly more common to me in general use (i.e. in javascript, in windowing frameworks, and in event frameworks). Those are usually handlers, not policy though.I think "On" reads naturally for handlers (i.e., we are describing the transition into the state, an edge) and "When" reads naturally for policies (we are describing the time during which it is in that state, a level).
----* I don't think we need to make these structs for tunables in the future because we can always add a struct that adds additional options alongside AND it's likely the tunables would be different by event type.
You received this message because you are subscribed to the Google Groups "kubernetes-api-reviewers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-api-rev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAH16ShK6Xh%3DE%2B5WzmqEKYMLenvObBFEmxT3kHq__fkBY_qs4ZQ%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "kubernetes-api-reviewers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-api-rev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAB_J3bazKp__1mCBhCZ2V%3Dgy7v%2BEW2mNNvUZdC2Z2ogpC_w-wQ%40mail.gmail.com.
On Wed, May 19, 2021 at 11:42 AM 'Daniel Smith' via kubernetes-api-reviewers <kubernetes-a...@googlegroups.com> wrote:On Wed, May 19, 2021 at 7:44 AM Clayton Coleman <ccol...@redhat.com> wrote:TL:DR: Establishing a novel convention about naming policy that occurs "on some change" that doesn't quite fit existing use.Wanted to suggest a new convention for naming for a specific use case - opinions welcome and desired:https://github.com/kubernetes/kubernetes/pull/99378 adds a policy struct PersistentVolumeClaimPolicy to statefulsets (probably to be renamed PersistentVolumeClaimRetentionPolicy for clarity).Within the struct there are two fields for the PVC retention policy on specific events/hooks - "deletion of the set" and "scale up or down" (deletion of a pod). It's a struct so we could in theory add more things later including both numeric properties (a delay?) or more events/hooks (hypothetical "on new revision".The original choice for names was `OnSetDeleted` and `OnScaleDown`. To me that's novel (the On*) pattern. We discussed omitting the prefix `Deleted` / `ScaleDown` but since this is a policy "in a circumstance" AND there might be other fields in here that weren't "in a circumstance" it was better to preface them with something.Existing art:TopologySpreadConstraint on pod uses `WhenUnsatisfiable` - describes the policy behavior in the context of other policy fields.Pod lifecycle hooks describe the event `PostStart`, `PreStop` - a hook is always done "at something" and so "When/On" would be redundant.I don't remember / couldn't find another example of "describing policy in the context of an event" EXCEPT for the general pattern of update, which is that we prefer to describe the policy in terms of rules only, not events (maxUnavailable).Proposed:* We need to give the field a prefix because there are potentially mixed "rules" and "conditional behavior" in the future in the struct. Also, I don't think a user would see "persistentVolumeClaimRetentionPolicy.deleted" in the object and automatically understand "oh, that's *when* it's deleted" - we have help them by being explicit. (following the lifecycle hook example is not clear to the user)* I think the prefix should be "When*" instead of "On*" because we at least use When today elsewhere and it reads more naturally to me "persistentVolumeClaimRetentionPolicy.whenDeleted = Reclaim". However, this is not backed up by prior art and "On*" is slightly more common to me in general use (i.e. in javascript, in windowing frameworks, and in event frameworks). Those are usually handlers, not policy though.I think "On" reads naturally for handlers (i.e., we are describing the transition into the state, an edge) and "When" reads naturally for policies (we are describing the time during which it is in that state, a level).Would we then say a "hook is a handler, but in a chunk of handlers where no other field type is possible we omit "on""? Another option is "handlers should ALWAYS be in their own grouped struct like pod lifecycle hooks" in which case we would always omit On*?
On Wed, May 19, 2021 at 11:46 AM Clayton Coleman <ccol...@redhat.com> wrote:On Wed, May 19, 2021 at 11:42 AM 'Daniel Smith' via kubernetes-api-reviewers <kubernetes-a...@googlegroups.com> wrote:On Wed, May 19, 2021 at 7:44 AM Clayton Coleman <ccol...@redhat.com> wrote:TL:DR: Establishing a novel convention about naming policy that occurs "on some change" that doesn't quite fit existing use.Wanted to suggest a new convention for naming for a specific use case - opinions welcome and desired:https://github.com/kubernetes/kubernetes/pull/99378 adds a policy struct PersistentVolumeClaimPolicy to statefulsets (probably to be renamed PersistentVolumeClaimRetentionPolicy for clarity).Within the struct there are two fields for the PVC retention policy on specific events/hooks - "deletion of the set" and "scale up or down" (deletion of a pod). It's a struct so we could in theory add more things later including both numeric properties (a delay?) or more events/hooks (hypothetical "on new revision".The original choice for names was `OnSetDeleted` and `OnScaleDown`. To me that's novel (the On*) pattern. We discussed omitting the prefix `Deleted` / `ScaleDown` but since this is a policy "in a circumstance" AND there might be other fields in here that weren't "in a circumstance" it was better to preface them with something.Existing art:TopologySpreadConstraint on pod uses `WhenUnsatisfiable` - describes the policy behavior in the context of other policy fields.Pod lifecycle hooks describe the event `PostStart`, `PreStop` - a hook is always done "at something" and so "When/On" would be redundant.I don't remember / couldn't find another example of "describing policy in the context of an event" EXCEPT for the general pattern of update, which is that we prefer to describe the policy in terms of rules only, not events (maxUnavailable).Proposed:* We need to give the field a prefix because there are potentially mixed "rules" and "conditional behavior" in the future in the struct. Also, I don't think a user would see "persistentVolumeClaimRetentionPolicy.deleted" in the object and automatically understand "oh, that's *when* it's deleted" - we have help them by being explicit. (following the lifecycle hook example is not clear to the user)* I think the prefix should be "When*" instead of "On*" because we at least use When today elsewhere and it reads more naturally to me "persistentVolumeClaimRetentionPolicy.whenDeleted = Reclaim". However, this is not backed up by prior art and "On*" is slightly more common to me in general use (i.e. in javascript, in windowing frameworks, and in event frameworks). Those are usually handlers, not policy though.I think "On" reads naturally for handlers (i.e., we are describing the transition into the state, an edge) and "When" reads naturally for policies (we are describing the time during which it is in that state, a level).Would we then say a "hook is a handler, but in a chunk of handlers where no other field type is possible we omit "on""? Another option is "handlers should ALWAYS be in their own grouped struct like pod lifecycle hooks" in which case we would always omit On*?Also, excellent use of on/when to describe edge vs level, policy is always level, kube especially prefers level, and edge is reserved for specific use cases.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAB_J3bZ1daqcx1POZc9MaTPCerO0B6xc2XLcW4Wh0_0Z8w0vtw%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-api-reviewers/CAO_RewbiGvsQ6E_GJ9DX4ir%3DcAV_h%3D2A7pgeJYqPzo313Njepg%40mail.gmail.com.