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

fix: show error on invalid import #14851

Merged
merged 4 commits into from May 27, 2021
Merged

Conversation

betodealmeida
Copy link
Member

SUMMARY

The current import API needs to raise an error if the user tries to import and invalid file.

Since we check which files are valid, I added a simple test to all import APIs:

if not contents:
    raise NoValidFilesFoundError()

Which returns an error message with a 400 HTTP status code.

I also took the time to SIP-41-fy the endpoints, removing the @safe decorator so exceptions can bubble up to the error handlers. Since the exceptions that the command can raise all have the correct HTTP status codes, this will preserve the existing behavior.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

Tested manually with an invalid import.

ADDITIONAL INFORMATION

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

@pull-request-size pull-request-size bot added size/L and removed size/M labels May 26, 2021
@betodealmeida betodealmeida merged commit 2313e3e into apache:master May 27, 2021
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
* fix: show error on invalid import

* Add unit test

* Remove unused imports

* Fix tests
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* fix: show error on invalid import

* Add unit test

* Remove unused imports

* Fix tests
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* fix: show error on invalid import

* Add unit test

* Remove unused imports

* Fix tests
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024
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 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants