fix(charts): Fix chart load task error handling

[giftig]

SUMMARY

The exceptions raised within this task are given internationalised messages (flask-babel), which means when the task tries to serialise the exception message to mark the job as failed, it needs to make sure they're rendered first. Cast to str to make that happen.

The underlying issue was using lazy_gettext when just using gettext would make more sense, so I've also adjusted the import in a couple of obvious places directly impacting this task to fix it at both ends.

I've left in the str cast fix as it'd require a much deeper dive to determine if any other edge cases would still result in a lazy message given the complexity of the task.

TESTING INSTRUCTIONS

See associated issue for full details.

1. Configure GLOBAL_ASYNC_QUERIES
2. Create a virtual dataset from any query
3. Edit the dataset to have invalid SQL, e.g. a syntax error
4. Click through to the charts page

ADDITIONAL INFORMATION

Fixes #24446

• Has associated issue: 2.1: No error reporting in charts with GLOBAL_ASYNC_QUERIES #24446
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #24447 (8337f31) into master (3e76736) will increase coverage by 0.08%.
The diff coverage is 88.35%.

❗ Current head 8337f31 differs from pull request most recent head 2f9d48a. Consider uploading reports for the commit 2f9d48a to get more accurate results

@@            Coverage Diff             @@
##           master   #24447      +/-   ##
==========================================
+ Coverage   68.91%   69.00%   +0.08%     
==========================================
  Files        1899     1901       +2     
  Lines       73843    74002     +159     
  Branches     8119     8116       -3     
==========================================
+ Hits        50892    51067     +175     
+ Misses      20840    20824      -16     
  Partials     2111     2111              

Flag	Coverage Δ
hive	53.93% <54.39%> (+0.03%)	⬆️
mysql	79.42% <89.12%> (+0.19%)	⬆️
postgres	79.51% <89.12%> (+0.19%)	⬆️
presto	53.83% <52.71%> (+0.02%)	⬆️
python	83.44% <90.79%> (+0.12%)	⬆️
sqlite	?
unit	54.68% <51.04%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ckages/superset-ui-chart-controls/src/constants.ts	100.00% <ø> (ø)
...erset-ui-chart-controls/src/utils/columnChoices.ts	100.00% <ø> (ø)
.../superset-ui-core/src/models/ExtensibleFunction.ts	100.00% <ø> (ø)
...packages/superset-ui-core/src/query/types/Query.ts	100.00% <ø> (ø)
superset-frontend/src/SqlLab/actions/sqlLab.js	69.16% <ø> (-0.27%)	⬇️
...d/src/SqlLab/components/SaveDatasetModal/index.tsx	50.00% <0.00%> (ø)
superset-frontend/src/SqlLab/fixtures.ts	100.00% <ø> (ø)
...rset-frontend/src/explore/components/SaveModal.tsx	35.08% <ø> (+0.30%)	⬆️
superset/databases/utils.py	82.22% <ø> (ø)
superset/models/sql_lab.py	79.51% <ø> (ø)
... and 33 more

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[john-bodley]

Thanks @giftig. Would you mind adding a unit test as well? This helps both with reviewing the PR and ensures we don't regress in the future.

[giftig]

@john-bodley yeah I see there's currently no coverage of this task at all; I'll take a look at writing something simple to cover this.

[giftig]

I've added a unit test covering the failure scenario specifically using lazy_gettext in the exception as that's a worst-case scenario, but it passes ok with gettext or a plain string too (I thought parametrize to cover all three scenarios would be a bit excessive coverage here)

[villebro]

LGTM, thanks for the added test here 👍