pbacsko commented on code in PR #957:
URL: https://github.com/apache/yunikorn-core/pull/957#discussion_r1745341074
##########
pkg/common/errors.go:
##########
@@ -24,4 +24,9 @@ import "errors"
var (
// InvalidQueueName returned when queue name is invalid
InvalidQueueName = errors.New("invalid queue name, max 64 characters
consisting of alphanumeric characters and '-', '_', '#', '@', '/', ':' allowed")
+
+ PreemptionPreconditionsFailed =
errors.New("PreemptionPreconditionsFailed")
+ PreemptionDoesNotGuarantee =
errors.New("PreemptionQueueGuaranteesCheckFailed")
+ PreemptionShortfall =
errors.New("PreemptionHelpedButShortfallOfResources")
+ PreemptionDoesNotHelp = errors.New("PreemptionDoesNotHelp")
Review Comment:
Just use string constants
##########
pkg/scheduler/objects/application.go:
##########
@@ -1122,6 +1123,7 @@ func (sa *Application) tryAllocate(headRoom
*resources.Resource, allowPreemption
return result
}
}
+
request.LogAllocationFailure(common.PreemptionDoesNotHelp.Error(), true)
Review Comment:
Please add proper coverage to this line
##########
pkg/scheduler/objects/preemption_test.go:
##########
@@ -85,6 +86,25 @@ func TestCheckPreconditions(t *testing.T) {
// verify that recently checked ask is disqualified
ask.preemptCheckTime = time.Now()
assert.Assert(t, !preemptor.CheckPreconditions(), "preconditions
succeeded with recently tried ask")
+ nodeMap := map[string]*Node{"node1": node}
+ getNode := func(nodeID string) *Node {
+ return nodeMap[nodeID]
Review Comment:
nit: do we need this map? Can't we just return a "node" object?
--
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]