aminghadersohi commented on code in PR #45819:
URL: https://github.com/apache/superset/pull/45819#discussion_r4407452494
##########
superset/tasks/scheduler.py:
##########
@@ -650,13 +650,25 @@ def _execute_task_body( # noqa: C901
set_ended_at=True,
).run()
- logger.error(
- "Task %s (uuid=%s) failed with error: %s",
- task_type,
- task_uuid,
- str(ex),
- exc_info=True,
- )
+ # Client-class errors (e.g. QueryObjectValidationError, status
400) are
Review Comment:
Nit: `SemanticResultCompletenessError("unverified")` inherits 400 and loses
its ERROR log traceback, although its guidance asks an administrator to check
the connection. Clarify this case's intended logging policy; the new logging
test covers only `"incomplete"`.
##########
superset/tasks/scheduler.py:
##########
@@ -650,13 +650,25 @@ def _execute_task_body( # noqa: C901
set_ended_at=True,
).run()
- logger.error(
- "Task %s (uuid=%s) failed with error: %s",
- task_type,
- task_uuid,
- str(ex),
- exc_info=True,
- )
+ # Client-class errors (e.g. QueryObjectValidationError, status
400) are
+ # user-fixable and already surfaced to the user via the task's
error
+ # properties; they are not server faults, so avoid ERROR +
traceback.
+ status = getattr(ex, "status", None)
+ if isinstance(status, int) and 400 <= status < 500:
Review Comment:
Nit: The PR description says non-Superset exceptions are unaffected, but
this condition accepts any exception with an integer 4xx `status`. Either scope
the check to `SupersetException` or document the broader contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
