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: add descriptive error msgs #1150

Merged

Conversation

lilyli9
Copy link
Contributor

@lilyli9 lilyli9 commented Feb 7, 2023

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
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

@@ -149,14 +150,41 @@ def _start_query_execution_task(
return query_execution.id


def get_datadoc_error_message(query_execution_id):
Copy link
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

querybook/server/tasks/run_datadoc.py Outdated Show resolved Hide resolved
)[0]
data_cell_name = datadoc_logic.get_data_cell_by_id(
data_cell_id, session=session
).meta.get("title")
Copy link
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

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
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

querybook/server/tasks/run_datadoc.py Show resolved Hide resolved
@lilyli9 lilyli9 requested a review from czgu February 17, 2023 23:52
@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
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

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
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}]")

)
query_execution_error_message = None
if query_execution_error:
if query_execution_error.error_message_extracted:
Copy link
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.

None yet

3 participants