Skip to content

feat: add descriptive error msgs - #1150

Merged
czgu merged 4 commits into
pinterest:masterfrom
lilyli9:external/descriptive-datadoc-error-msgs
Feb 24, 2023
Merged

czgu merged 4 commits into
pinterest:masterfrom
lilyli9:external/descriptive-datadoc-error-msgs

Conversation

@lilyli9

@lilyli9 lilyli9 commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

Adds more descriptive error messages when running a datadoc fails. This error message contains the exact error from the query cell, and the name of the failed query cell to help pinpoint the location of the error.

Screen Shot 2023-02-07 at 2 51 54 PM

Comment on lines 126 to +127
if previous_query_status != QueryExecutionStatus.DONE.value:
raise Exception(GENERIC_QUERY_FAILURE_MSG)
raise Exception(get_datadoc_error_message(previous_query_execution_id))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@czgu do you know why we raise the exception in the next query cell run task, instead of just raising it at the end of the run_query task? In which case we dont need to pass over the previous query result over and can throw the error message directly

Comment thread querybook/server/tasks/run_datadoc.py Outdated
return query_execution.id


def get_datadoc_error_message(query_execution_id):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use @with_session instead

Comment thread querybook/server/tasks/run_datadoc.py Outdated
Comment thread querybook/server/tasks/run_datadoc.py Outdated
)[0]
data_cell_name = datadoc_logic.get_data_cell_by_id(
data_cell_id, session=session
).meta.get("title")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

title might be empty, can you add a default value here

Comment thread querybook/server/tasks/run_datadoc.py Outdated
data_cell_name = datadoc_logic.get_data_cell_by_id(
data_cell_id, session=session
).meta.get("title")
query_execution_error = qe_logic.get_query_execution_by_id(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use get_query_execution_error

Comment thread querybook/server/tasks/run_datadoc.py
@lilyli9
lilyli9 requested a review from czgu February 17, 2023 23:52
Comment thread querybook/server/tasks/run_datadoc.py Outdated
@with_session
def get_datadoc_error_message(query_execution_id, data_cell_id=None, session=None):
if not data_cell_id:
_, data_cell_id = qe_logic.get_datadoc_id_from_query_execution_id(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry for my last comment, I just realized I made a mistake because you need the previous cell id, not the current one

Comment thread querybook/server/tasks/run_datadoc.py Outdated
data_cell_name = datadoc_logic.get_data_cell_by_id(
data_cell_id, session=session
).meta.get("title")
if not data_cell_name:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

data_cell_name = datadoc_logic.get_data_cell_by_id(
        data_cell_id, session=session
    ).meta.get("title", f"Untitled Cell Id [{data_cell_id}]")

Comment thread querybook/server/tasks/run_datadoc.py Outdated
)
query_execution_error_message = None
if query_execution_error:
if query_execution_error.error_message_extracted:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

query_execution_error_message = query_execution_error.error_message_extracted if query_execution_error.error_message_extracted else query_execution_error.error_message

error_msg = (f'Failure in "{data_cell_name}": {query_execution_error_message}'
        if query_execution_error_message is not None
        else GENERIC_QUERY_FAILURE_MSG
)[:description_len]

@lilyli9
lilyli9 requested a review from czgu February 22, 2023 19:08
@czgu
czgu merged commit 8ab460f into pinterest:master Feb 24, 2023
rohan-sh1 pushed a commit to CAI-TECHNOLOGIES/cai-ext-db-explorer that referenced this pull request Apr 11, 2023
* feat: add descriptive error msgs

* fix: only get error msg if exec error

* chore: refactor get_datadoc_error_message

* chore: refactor helper func, del data_cell_id arg
aidenprice pushed a commit to arrowtail-precision/querybook that referenced this pull request Jan 3, 2024
* feat: add descriptive error msgs

* fix: only get error msg if exec error

* chore: refactor get_datadoc_error_message

* chore: refactor helper func, del data_cell_id arg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants