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.

Sign up for GitHub

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

Jump to bottom

fix(sql lab): Syntax errors should return with 422 status #20491

Merged
merged 2 commits into from
Jul 27, 2022
Merged

fix(sql lab): Syntax errors should return with 422 status #20491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf64245
fix(sql lab): Syntax errors should return with 422 status
diegomedina248 Jun 24, 2022
9fb87b1
refactor
diegomedina248 Jun 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions superset/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ def __init__(
self.status = status


class SupersetSyntaxErrorException(SupersetErrorsException):
status = 422
error_type = SupersetErrorType.SYNTAX_ERROR

def __init__(self, errors: List[SupersetError]) -> None:
super().__init__(errors)


class SupersetTimeoutException(SupersetErrorFromParamsException):
status = 408

Expand Down
25 changes: 22 additions & 3 deletions superset/sqllab/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@
from superset.commands.base import BaseCommand
from superset.common.db_query_status import QueryStatus
from superset.dao.exceptions import DAOCreateFailedError
from superset.errors import SupersetErrorType
from superset.exceptions import SupersetErrorsException, SupersetGenericErrorException
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import (
SupersetErrorsException,
SupersetException,
SupersetGenericErrorException,
SupersetSyntaxErrorException,
)
from superset.models.core import Database
from superset.models.sql_lab import Query
from superset.sqllab.command_status import SqlJsonExecutionStatus
Expand Down Expand Up @@ -110,7 +115,21 @@ def run( # pylint: disable=too-many-statements,useless-suppression
"status": status,
"payload": self._execution_context_convertor.serialize_payload(),
}
except (SqlLabException, SupersetErrorsException) as ex:
except SupersetErrorsException as ex:
if all(ex.error_type == SupersetErrorType.SYNTAX_ERROR for ex in ex.errors):
raise SupersetSyntaxErrorException(ex.errors) from ex
raise ex
except SupersetException as ex:
Copy link
Member

Choose a reason for hiding this comment

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

I noticed that we were catching SqlLabException before. This one is a parent class, correct, so it should still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, correct, and most likely that exception will be thrown as is, unless it matches the criteria

if ex.error_type == SupersetErrorType.SYNTAX_ERROR:
raise SupersetSyntaxErrorException(
[
SupersetError(
message=ex.message,
error_type=ex.error_type,
level=ErrorLevel.ERROR,
)
]
) from ex
raise ex
except Exception as ex:
raise SqlLabException(self._execution_context, exception=ex) from ex
Expand Down