wilfred-s commented on code in PR #830:
URL: https://github.com/apache/yunikorn-core/pull/830#discussion_r1629051527
##########
pkg/common/resources/resources.go:
##########
@@ -645,6 +645,23 @@ func Equals(left, right *Resource) bool {
return true
}
+// MatchAnyOnlyExisting Is there at least one resource type match between left
& right resources?
+// Matching happens only for the resource existing only in left but not vice
versa.
+func MatchAnyOnlyExisting(left, right *Resource) bool {
Review Comment:
I think this signature should be `func (r *Resource) MatchAny(other
*Resource) bool`
With the description:
```
// MatchAny returns true if at least one type in the defined resource exists
in the other resource.
// False if none of the types exist in the other resource.
// A nil resource is treated as an empty resource (no types defined) and
returns false
// Values are not considered during the checks
```
##########
pkg/common/resources/resources.go:
##########
@@ -751,6 +768,47 @@ func StrictlyGreaterThanOrEquals(larger, smaller
*Resource) bool {
return true
}
+// StrictlyGreaterThanOnlyExisting Return true if all quantities present or
existing only in larger > smaller
+// Two resources that are equal are not considered strictly larger than each
other.
+// Resource present in smaller but not in larger are not even considered.
+func StrictlyGreaterThanOnlyExisting(larger, smaller *Resource) bool {
+ if larger == nil {
+ larger = Zero
+ }
+ if smaller == nil {
+ smaller = Zero
+ }
+
+ // keep track of the number of not equal values
+ notEqual := false
+
+ // Is larger and smaller completely disjoint?
+ atleastOneResourcePresent := false
+ for k, v := range larger.Resources {
+ // even when smaller is empty, at least one of the resource
type in larger should be greater than zero
+ if smaller.IsEmpty() && v > 0 {
+ return true
+ }
Review Comment:
if I have `{larger: {first: -10, second: 5}}` with an empty smaller this
call will return `true`
That does not seem correct. It is not "strict" in that case.
##########
pkg/common/resources/resources.go:
##########
@@ -751,6 +768,47 @@ func StrictlyGreaterThanOrEquals(larger, smaller
*Resource) bool {
return true
}
+// StrictlyGreaterThanOnlyExisting Return true if all quantities present or
existing only in larger > smaller
+// Two resources that are equal are not considered strictly larger than each
other.
+// Resource present in smaller but not in larger are not even considered.
+func StrictlyGreaterThanOnlyExisting(larger, smaller *Resource) bool {
Review Comment:
I think this signature should be `func (r *Resource)
StrictlyGreaterThanOnlyExisting(smaller *Resource) bool`
With the description:
```
// StrictlyGreaterThanOnlyExisting returns true if all quantities for types
in the defined resource are greater than
// the quantity for the same type in smaller.
// Types defined in smaller that are not in the defined resource are ignored.
// Two resources that are equal are not considered strictly larger than each
other.
```
For consistency we should add and implement: if smaller is nil or empty all
quantities in the defined resource must be greater than 0 (see below comment)
##########
pkg/common/resources/resources.go:
##########
@@ -816,6 +874,29 @@ func ComponentWiseMinPermissive(left, right *Resource)
*Resource {
return out
}
+// ComponentWiseMinOnlyExisting Returns a new Resource with the smallest value
for resource type
+// existing only in left but not vice versa.
+func ComponentWiseMinOnlyExisting(left, right *Resource) *Resource {
Review Comment:
This is the 3rd version of `ComponentWiseMin...` need to think about a
refactor in a follow up jira
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
