wilfred-s commented on code in PR #740:
URL: https://github.com/apache/yunikorn-core/pull/740#discussion_r1408672388
##########
pkg/scheduler/tests/recovery_test.go:
##########
Review Comment:
unneeded change for this jira
##########
pkg/scheduler/objects/allocation.go:
##########
@@ -295,6 +299,12 @@ func (a *Allocation) GetUUID() string {
return a.uuid
}
+// SetUUID set the uuid for this allocation
+// only for tests
+func (a *Allocation) SetUUID(uuid string) {
Review Comment:
Rename to `SetAllocationID` and the parameter rename to `id`.
Move away from the UUID as mentioned by Peter
##########
pkg/scheduler/objects/allocation.go:
##########
@@ -92,7 +92,7 @@ func NewAllocation(uuid, nodeID string, ask *AllocationAsk)
*Allocation {
bindTime: time.Now(),
nodeID: nodeID,
partitionName:
common.GetPartitionNameWithoutClusterID(ask.GetPartitionName()),
- uuid: uuid,
+ uuid: ask.allocationKey + "-" +
strconv.Itoa(ask.completedPendingAsk()),
Review Comment:
As a preparation add a new `GetAllocationID` function with the same
functionality as the `GetUUID` function in this change. Start calling that
instead of introducing 60+ new calls to `GetUUID` to simplify and reduce churn
in the follow up.
##########
pkg/scheduler/objects/allocation.go:
##########
@@ -152,7 +154,9 @@ func NewAllocationFromSI(alloc *si.Allocation) *Allocation {
createTime: time.Unix(creationTime, 0),
allocLog: make(map[string]*AllocationLogEntry),
}
- return NewAllocation(alloc.UUID, alloc.NodeID, ask)
+ newAlloc := NewAllocation(alloc.NodeID, ask)
+ newAlloc.uuid = alloc.UUID
+ return newAlloc
Review Comment:
Not sure that this is needed. Recovered allocation were already using the
pod UID. The only difference would be that a recovered allocation would become
<pod UID>-1 instead of <pod-UID>-0 for normal allocations.
I do not see the harm in that. There will be a difference anyway as now we
just have <pod-UID> without the trailing number.
##########
pkg/scheduler/tests/mock_rm_callback_test.go:
##########
Review Comment:
unneeded change for this 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: reviews-unsubscr...@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
