Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: generic marshmallow error component #25303

Merged
merged 6 commits into from
Oct 3, 2023
Merged

feat: generic marshmallow error component #25303

merged 6 commits into from
Oct 3, 2023

Conversation

betodealmeida
Copy link
Member

SUMMARY

Add better error handling for Marshmallow errors, with a custom exception, error, and React component.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Clipboard 2023-06-06 at 4 26 41 PM

After:

Screenshot 2023-09-11 at 15-55-55 Superset

Screenshot 2023-09-11 at 15-00-07 Superset

TESTING INSTRUCTIONS

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

@john-bodley john-bodley added the review:checkpoint Last PR reviewed during the daily review standup label Sep 14, 2023
@michael-s-molina michael-s-molina removed the review:checkpoint Last PR reviewed during the daily review standup label Sep 18, 2023
@eschutho
Copy link
Member

This is a really great improvement @betodealmeida!

@yousoph
Copy link
Member

yousoph commented Sep 29, 2023

/testenv up

@github-actions
Copy link
Contributor

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

@betodealmeida betodealmeida force-pushed the sc_71594 branch 2 times, most recently from 66fc17e to e62bea2 Compare October 3, 2023 00:24
@@ -88,7 +89,7 @@ export type ErrorType = ValueOf<typeof ErrorTypeEnum>;
// Keep in sync with superset/views/errors.py
export type ErrorLevel = 'info' | 'warning' | 'error';

export type ErrorSource = 'dashboard' | 'explore' | 'sqllab';
export type ErrorSource = 'dashboard' | 'explore' | 'sqllab' | 'crud';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crud doesn't seem like a domain source like others here. Is there a better name for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I couldn't think of a better name. The CRUD is one of the main domains, just not a fancy one. :-P

Copy link
Contributor

@zephyring zephyring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM one minor comment

@betodealmeida betodealmeida merged commit 3e63c82 into master Oct 3, 2023
54 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Ephemeral environment shutdown and build artifacts deleted.

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ 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
@mistercrunch mistercrunch deleted the sc_71594 branch March 26, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants