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

Allow Looker API errors with a single, string-typed error #718

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 20 additions & 5 deletions spectacles/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,20 @@ def runtime(self) -> float:


class ErrorQueryResult(BaseModel):
class QueryResultData(BaseModel):
class ErrorData(BaseModel):
id: str
error: str
runtime = 0.0
sql = ""

class MultiErrorData(BaseModel):
id: str
runtime: float
sql: str
errors: Optional[Tuple[QueryError, ...]]

status: Literal["error"]
data: QueryResultData
data: Union[ErrorData, MultiErrorData]

@property
def runtime(self) -> float:
Expand All @@ -71,9 +77,18 @@ def sql(self) -> str:

@property
def errors(self) -> Tuple[QueryError, ...]:
if self.data.errors is None:
raise TypeError("No errors contained in this query result")
return self.data.errors
if isinstance(self.data, self.ErrorData):
return (
QueryError(
message=self.data.error, message_details=None, sql_error_loc=None
),
)
elif isinstance(self.data, self.MultiErrorData):
if self.data.errors is None:
raise TypeError("No errors contained in this query result")
return self.data.errors
else:
raise TypeError("Unexpected type for ErrorQueryResult.data")

def get_valid_errors(self) -> Tuple[QueryError, ...]:
WARNINGS = (
Expand Down
26 changes: 18 additions & 8 deletions spectacles/validators/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@
from dataclasses import dataclass
import time
from tabulate import tabulate
from typing import List, Optional, Tuple, Iterator, Union, cast
from typing import List, Optional, Tuple, Iterator
import pydantic
from spectacles.client import LookerClient
from spectacles.lookml import CompiledSql, Dimension, Explore
from spectacles.exceptions import SpectaclesException, SqlError
from spectacles.logger import GLOBAL_LOGGER as logger
from spectacles.printer import print_header
from spectacles.utils import consume_queue, halt_queue
from spectacles.types import QueryResult, CompletedQueryResult, ErrorQueryResult
from spectacles.types import (
QueryResult,
CompletedQueryResult,
ErrorQueryResult,
InterruptedQueryResult,
)

QUERY_TASK_LIMIT = 250
DEFAULT_CHUNK_SIZE = 500
Expand Down Expand Up @@ -316,10 +321,9 @@ async def _get_query_results(
)

# Append long-running queries for the profiler
if query_result.status in ("complete", "error"):
query_result = cast(
Union[CompletedQueryResult, ErrorQueryResult], query_result
)
if isinstance(
query_result, (CompletedQueryResult, ErrorQueryResult)
):
query = self._task_to_query[task_id]
query.runtime = query_result.runtime
if query_result.runtime > self.runtime_threshold:
Expand Down Expand Up @@ -393,7 +397,10 @@ async def _get_query_results(
# Indicate there are no more queries or subqueries to run
queries_to_run.task_done()

elif query_result.status == "killed":
elif (
isinstance(query_result, InterruptedQueryResult)
and query_result.status == "killed"
):
query = self._task_to_query[task_id]
query.errored = True
explore = query.explore
Expand All @@ -416,7 +423,10 @@ async def _get_query_results(
query_slot.release()
queries_to_run.task_done()

elif query_result.status == "expired":
elif (
isinstance(query_result, InterruptedQueryResult)
and query_result.status == "expired"
):
query = self._task_to_query[task_id]
query.expired_at = query.expired_at or time.time()
expired_for = time.time() - query.expired_at
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_sql_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ async def test_get_query_results_handles_bogus_expired_queries(
json={
query_task_id: ErrorQueryResult(
status="error",
data=ErrorQueryResult.QueryResultData(
data=ErrorQueryResult.MultiErrorData(
id="",
runtime=1.0,
sql="",
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,21 @@ def test_get_valid_errors_should_ignore_warnings():
query_result = cast(ErrorQueryResult, QueryResult.parse_obj(response_json).__root__)
valid_errors = query_result.get_valid_errors()
assert not valid_errors


def test_can_parse_string_errors():
response = {
"status": "error",
"result_source": None,
"data": {
"from_cache": True,
"id": "67120bd3c7d23eb81f72692be9581c4a",
"error": "View Not Found",
},
}

result = QueryResult.parse_obj(response).__root__
assert isinstance(result, ErrorQueryResult)
assert result.errors[0].message == "View Not Found"
assert result.runtime == 0.0
assert result.sql == ""