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

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

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


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

Review Comment:
   Good idea. I'm still contemplating different solutions to this problem, as there's multiple things that are wrong right now:
   1. The absence of a locale in celery tasks (which this is primarily trying to address)
   2. Incorrect use of `lazy_gettext` where `gettext` is sufficient
   3. serializing/deserializing using `json`.
   
   Thinking more about this I'm not so sure anymore if we should really be using `flask.json` as I've done here (bullet nr 3). So maybe that part should be reverted, and also that error message logic DRY'd up like you are proposing.



-- 
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