You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/03/05 05:52:24 UTC

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #376: [YUNIKORN-1101] idempotent remove application and remove task in context

0yukali0 commented on a change in pull request #376:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/376#discussion_r820049701



##########
File path: pkg/cache/context.go
##########
@@ -660,11 +660,8 @@ func (ctx *Context) RemoveApplication(appID string) error {
 func (ctx *Context) RemoveApplicationInternal(appID string) error {
 	ctx.lock.Lock()
 	defer ctx.lock.Unlock()
-	if _, exist := ctx.applications[appID]; exist {
-		delete(ctx.applications, appID)
-		return nil
-	}
-	return fmt.Errorf("application %s is not found in the context", appID)
+	delete(ctx.applications, appID)

Review comment:
       I think we can treat the message that shows the application doesn't exist as a log information.
   Like you did in RemoveTask function.
   `
   if err != nil {
     log.Logger().Info(" not found, it has been deleted",
     zap.String("Application", appID))
     }
   `




-- 
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-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org