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