You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "giftig (via GitHub)" <gi...@apache.org> on 2023/06/20 14:12:17 UTC

[GitHub] [superset] giftig commented on a diff in pull request #24457: [WIP] fix(gaq): support localized error messages

giftig commented on code in PR #24457:
URL: https://github.com/apache/superset/pull/24457#discussion_r1235333260


##########
superset/tasks/async_queries.py:
##########
@@ -96,15 +101,13 @@ def load_chart_data_into_cache(
                 else str(ex)
             )
             errors = [{"message": error}]

Review Comment:
   I wonder if this code is repeated; feels like I've seen errors reported as `{"message": "(some exception message)"}` fairly frequently so maybe it could be more DRY and be made safe against lazy messaging for good by reusing any logic we need surrounding that.
   
   Presumably even with locale here, we still need to be using `gettext` instead of `gettext_lazy` when raising the error we've caught here; having the locale context will let us translate the message correctly but won't fix the fact that the `json` library doesn't know how to serialise `LazyString`.



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

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


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