john-bodley commented on code in PR #24607:
URL: https://github.com/apache/superset/pull/24607#discussion_r1254919560


##########
superset/annotation_layers/annotations/commands/exceptions.py:
##########
@@ -68,5 +64,5 @@ class AnnotationUpdateFailedError(CreateFailedError):
     message = _("Annotation could not be updated.")
 
 
-class AnnotationDeleteFailedError(CommandException):

Review Comment:
   This illustrates a great example of the benefits of condensing logic. The 
bulk exceptions:
   
   1. Is subclassed from  `DeleteFailedError`
   2. Uses language of the form `Annotations could not be deleted.`
   
   whereas the non-bulk exceptions:
   
   1. Is subclassed from `CommandException`.
   2. Uses language of the form  `Annotation delete failed.`
   
   Here the legacy bulk logic seems more correct in terms of consistency with 
how create, update, etc. exceptions are defined.



-- 
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: notifications-unsubscr...@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org

Reply via email to