fix: allow for backward compatible errors #25640

eschutho · 2023-10-14T01:21:15Z

SUMMARY

This adds some backward compatibility to the new marshmallow errors for the dataset editor when the errors are not SIP40 compliant.
I also created a new default value that we can pass to new registries and used the default error message for the error message registry.
Lastly I also converted some tests to RTL.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

Create a dataset with two calculated columns of the same name.

ADDITIONAL INFORMATION

Has associated issue:
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

betodealmeida · 2023-10-25T22:33:48Z

superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx

-    const table = getByTestId('table-container');
-    expect(table).toBeInTheDocument();
-  });
+// describe('ResultSet', () => {


Do we need to comment all of this out? If you do:

Suggested change

// describe('ResultSet', () => {

describe.skip('ResultSet', () => {

It will skip all the tests.

Thanks, yeah I'm just trying to test to see which test is OOMing. :|

I'll revert this once I have it solved. 👍

betodealmeida · 2023-10-25T22:34:46Z

superset-frontend/src/components/Datasource/DatasourceModal.tsx

+        getClientErrorObject(response).then(error => {
+          let errorResponse: SupersetError | undefined;
+          let errorText: string | undefined;
+          // sip-40 error response
+          if (error?.errors?.length) {
+            errorResponse = error.errors[0];
+          } else if (typeof error.error === 'string') {
+            // backward compatible with old error messages
+            errorText = error.error;
+          }


eschutho · 2023-10-26T16:30:02Z

/testenv up

github-actions · 2023-10-26T16:34:17Z

@eschutho Ephemeral environment spinning up at http://35.85.217.235:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

betodealmeida · 2023-10-26T23:01:11Z

✅ looks great!

jinghua-qa · 2023-10-27T15:53:05Z

I saw empty db error when i tried to add a duplicated column in the ephemeral env, should i expect to see the error for "one or more columns are duplicated?"

eschutho · 2023-10-27T17:15:10Z

/testenv up

github-actions · 2023-10-27T17:19:04Z

@eschutho Ephemeral environment spinning up at http://34.212.189.236:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

eschutho · 2023-10-27T17:23:53Z

@jinghua-qa I think the last ephemeral didn't have all of the changes reflected, so I'm spinning up a new one. Thanks!

jinghua-qa · 2023-10-27T17:50:27Z

Tested again and saw the correct screen for error msg~ LGTM

jinghua-qa

LGTM

github-actions · 2023-10-27T18:05:03Z

Ephemeral environment shutdown and build artifacts deleted.

(cherry picked from commit ed14f36)

sadpandajoe · 2023-10-27T19:50:28Z

🏷️ preset:2023.43

* fix(sqllab): reinstate "Force trino client async execution" (apache#25680) * fix: remove unnecessary redirect (apache#25679) (cherry picked from commit da42bf2) * fix(chore): dashboard requests to database equal the number of slices it has (apache#24709) (cherry picked from commit 75a7431) * fix: bump to FAB 4.3.9 remove CSP exception (apache#25712) (cherry picked from commit 8fb0c8d) * fix(horizontal filter label): show full tooltip with ellipsis (apache#25732) (cherry picked from commit e4173d9) * fix: Revert "fix(Charts): Set max row limit + removed the option to use an empty row limit value" (apache#25753) (cherry picked from commit e2fe967) * fix: dataset update uniqueness (apache#25756) (cherry picked from commit c7f8d11) * fix(sqllab): slow pop datasource query (apache#25741) (cherry picked from commit 2a2bc82) * fix: allow for backward compatible errors (apache#25640) * fix: DB-specific quoting in Jinja macro (apache#25779) (cherry picked from commit 5659c87) * fix: Revert "fix: Apply normalization to all dttm columns (apache#25147)" (apache#25801) * fix: Resolve issue apache#24195 (apache#25804) (cherry picked from commit 8737a8a) * fix(SQL field in edit dataset modal): display full sql query (apache#25768) (cherry picked from commit 1eba712) * fix(sqllab): infinite fetching status after results are landed (apache#25814) (cherry picked from commit 3f28eeb) * fix: Fires onChange when clearing all values of single select (apache#25853) (cherry picked from commit 8061d5c) * fix: the temporal x-axis results in a none time_range. (apache#25429) Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> (cherry picked from commit ae619b1) * fix(table chart): Show Cell Bars correctly apache#25625 (apache#25707) (cherry picked from commit 916f7bc) * fix: remove `update_charts_owners` (apache#25843) * fix(charts): Time grain is None when dataset uses Jinja (apache#25842) (cherry picked from commit 7536dd1) * fix: Saving Mixed Chart with dashboard filter applied breaks adhoc_filter_b (apache#25877) (cherry picked from commit 268c1dc) * fix: database version field (apache#25898) (cherry picked from commit 06ffcd2) * fix: trino cursor (apache#25897) (cherry picked from commit cdb18e0) * chore: Updates CHANGELOG.md for 3.0.2 * fix(trino): allow impersonate_user flag to be imported (apache#25872) Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com> (cherry picked from commit 458be8c) * fix(table): Double percenting ad-hoc percentage metrics (apache#25857) (cherry picked from commit 784a478) * fix(sqllab): invalid sanitization on comparison symbol (apache#25903) (cherry picked from commit 581d3c7) * fix: update flask-caching to avoid breaking redis cache, solves apache#25339 (apache#25947) Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * fix: always denorm column value before querying values (apache#25919) * chore(colors): Updating Airbnb brand colors (apache#23619) (cherry picked from commit 6d8424c) * fix: naming denomalized to denormalized in helpers.py (apache#25973) (cherry picked from commit 5def416) * fix(helm): Restart all related deployments when bootstrap script changed (apache#25703) * fix(rls): Update text from tables to datasets in RLS modal (apache#25997) (cherry picked from commit 210f1f8) * fix: Make Select component fire onChange listener when a selection is pasted in (apache#25993) (cherry picked from commit 5fccf67) * fix(explore): redandant force param (apache#25985) (cherry picked from commit e7a1876) * chore: Optimize fetching samples logic (apache#25995) (cherry picked from commit 326ac4a) * fix(native filters): rendering performance improvement by reduce overrendering (apache#25901) (cherry picked from commit e1d73d5) * fix: update FAB to 4.3.10, Azure user info fix (apache#26037) (cherry picked from commit 628cd34) * chore: Updates CHANGELOG.md for 3.0.2 (rc2) --------- Co-authored-by: Rob Moore <giftig@users.noreply.github.com> Co-authored-by: Igor Khrol <igor.khrol@automattic.com> Co-authored-by: Stepan <66589759+Always-prog@users.noreply.github.com> Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com> Co-authored-by: Ross Mabbett <92495987+rtexelm@users.noreply.github.com> Co-authored-by: Geido <60598000+geido@users.noreply.github.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: mapledan <mapledan829@gmail.com> Co-authored-by: Arko <90512504+SA-Ark@users.noreply.github.com> Co-authored-by: Antonio Rivero <38889534+Antonio-RiveroMartnez@users.noreply.github.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com> Co-authored-by: FGrobelny <150029280+FGrobelny@users.noreply.github.com> Co-authored-by: Giacomo Barone <46573388+ggbaro@users.noreply.github.com> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> Co-authored-by: josedev-union <70741025+josedev-union@users.noreply.github.com> Co-authored-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Jack Fragassi <jfragassi98@gmail.com>

pull-request-size bot added the size/M label Oct 14, 2023

eschutho force-pushed the elizabeth/fix-error-message-dataset branch 4 times, most recently from b17f4b1 to ecdec2e Compare October 17, 2023 00:26

pull-request-size bot added size/L and removed size/M labels Oct 18, 2023

eschutho changed the title ~~allow for backward compatible errors~~ fix: allow for backward compatible errors Oct 19, 2023

eschutho marked this pull request as ready for review October 19, 2023 00:02

eschutho force-pushed the elizabeth/fix-error-message-dataset branch 2 times, most recently from 35e399c to 18bb997 Compare October 19, 2023 01:31

eschutho added 2 commits October 25, 2023 09:28

allow for backward compatible errors

2e91e0d

update tests

906dced

eschutho force-pushed the elizabeth/fix-error-message-dataset branch from 18bb997 to 906dced Compare October 25, 2023 16:28

pull-request-size bot added size/XL and removed size/L labels Oct 25, 2023

betodealmeida approved these changes Oct 25, 2023

View reviewed changes

eschutho force-pushed the elizabeth/fix-error-message-dataset branch 2 times, most recently from e183e76 to 9fb0e26 Compare October 26, 2023 01:03

pull-request-size bot added size/L and removed size/XL labels Oct 26, 2023

eschutho force-pushed the elizabeth/fix-error-message-dataset branch from 9fb0e26 to c333898 Compare October 26, 2023 01:06

scope down default error

a917225

eschutho force-pushed the elizabeth/fix-error-message-dataset branch from c333898 to a917225 Compare October 26, 2023 18:35

jinghua-qa approved these changes Oct 27, 2023

View reviewed changes

eschutho merged commit ed14f36 into apache:master Oct 27, 2023
26 checks passed

eschutho deleted the elizabeth/fix-error-message-dataset branch October 27, 2023 18:04

sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Oct 27, 2023

fix: allow for backward compatible errors (apache#25640)

1bd6b18

(cherry picked from commit ed14f36)

michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Oct 31, 2023

michael-s-molina pushed a commit that referenced this pull request Oct 31, 2023

fix: allow for backward compatible errors (#25640)

2f46890

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

fix: allow for backward compatible errors (apache#25640)

7a713e5

mistercrunch added 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024

sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 2024

fix: allow for backward compatible errors (apache#25640)

4af082d

vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024

fix: allow for backward compatible errors (apache#25640)

ee42863

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow for backward compatible errors #25640

fix: allow for backward compatible errors #25640

eschutho commented Oct 14, 2023 •

edited

Loading

betodealmeida Oct 25, 2023

eschutho Oct 25, 2023

eschutho Oct 25, 2023

betodealmeida Oct 25, 2023

eschutho commented Oct 26, 2023

github-actions bot commented Oct 26, 2023

betodealmeida commented Oct 26, 2023

jinghua-qa commented Oct 27, 2023

eschutho commented Oct 27, 2023

github-actions bot commented Oct 27, 2023

eschutho commented Oct 27, 2023

jinghua-qa commented Oct 27, 2023

jinghua-qa left a comment

github-actions bot commented Oct 27, 2023

sadpandajoe commented Oct 27, 2023

	// describe('ResultSet', () => {
	describe.skip('ResultSet', () => {

fix: allow for backward compatible errors #25640

fix: allow for backward compatible errors #25640

Conversation

eschutho commented Oct 14, 2023 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

betodealmeida Oct 25, 2023

Choose a reason for hiding this comment

eschutho Oct 25, 2023

Choose a reason for hiding this comment

eschutho Oct 25, 2023

Choose a reason for hiding this comment

betodealmeida Oct 25, 2023

Choose a reason for hiding this comment

eschutho commented Oct 26, 2023

github-actions bot commented Oct 26, 2023

betodealmeida commented Oct 26, 2023

jinghua-qa commented Oct 27, 2023

eschutho commented Oct 27, 2023

github-actions bot commented Oct 27, 2023

eschutho commented Oct 27, 2023

jinghua-qa commented Oct 27, 2023

jinghua-qa left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 27, 2023

sadpandajoe commented Oct 27, 2023

eschutho commented Oct 14, 2023 •

edited

Loading