Skip to content

Commit

Permalink
Merge pull request #706 from spectacles-ci/josh/eng-84-profiling-on-c…
Browse files Browse the repository at this point in the history
…ore-does-not-work

Fix profiling and open it back up to all queries
  • Loading branch information
joshtemple authored Apr 19, 2023
2 parents bdd7123 + 925649e commit 5bdea9d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 6 deletions.
4 changes: 1 addition & 3 deletions spectacles/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,7 @@ async def validate_sql(
)

async with self.branch_manager(ref=ref):
await validator.search(
explores, fail_fast, chunk_size, profile=profile if fail_fast else False
)
await validator.search(explores, fail_fast, chunk_size, profile=profile)

# Create dimension tests for the desired ref when explores errored
if not fail_fast and incremental:
Expand Down
12 changes: 10 additions & 2 deletions spectacles/validators/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
DEFAULT_CHUNK_SIZE = 500
DEFAULT_QUERY_CONCURRENCY = 10
DEFAULT_RUNTIME_THRESHOLD = 5
ProfilerTableRow = Tuple[str, float, str, str]
ProfilerTableRow = Tuple[str, str, float, str, str]


@dataclass
Expand Down Expand Up @@ -62,7 +62,13 @@ def to_profiler_format(self) -> ProfilerTableRow:
"Query.explore_url cannot be None, "
"run Query.create to get an explore URL"
)
return (self.explore.name, self.runtime, self.query_id, self.explore_url)
return (
self.explore.name,
self.dimensions[0].name if len(self.dimensions) == 1 else "*",
self.runtime,
self.query_id,
self.explore_url,
)


def print_profile_results(queries: List[Query], runtime_threshold: int) -> None:
Expand All @@ -79,6 +85,7 @@ def print_profile_results(queries: List[Query], runtime_threshold: int) -> None:
[query.to_profiler_format() for query in queries_by_runtime],
headers=[
"Explore",
"Dimension(s)",
"Runtime (s)",
"Query ID",
"Explore From Here",
Expand Down Expand Up @@ -315,6 +322,7 @@ async def _get_query_results(
Union[CompletedQueryResult, ErrorQueryResult], query_result
)
query = self._task_to_query[task_id]
query.runtime = query_result.runtime
if query_result.runtime > self.runtime_threshold:
self._long_running_queries.append(query)
if query_result.status == "complete":
Expand Down
8 changes: 8 additions & 0 deletions tests/integration/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,11 @@ async def test_incremental_sql_with_diff_explores_and_invalid_existing_sql_shoul
assert result["tested"][1]["explore"] == "users__fail"
assert result["tested"][1]["status"] == "passed"
assert len(result["errors"]) == 0


async def test_validate_sql_with_query_profiler_should_work(
looker_client: LookerClient, caplog: pytest.LogCaptureFixture
):
runner = Runner(looker_client, "eye_exam")
await runner.validate_sql(fail_fast=True, profile=True, runtime_threshold=0)
assert "Query profiler results" in caplog.text
23 changes: 22 additions & 1 deletion tests/unit/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,25 @@ def test_query_should_convert_to_profiler_format(
query_id=query_id,
explore_url=explore_url,
)
assert query.to_profiler_format() == (explore.name, runtime, query_id, explore_url)
assert query.to_profiler_format() == (
explore.name,
"*",
runtime,
query_id,
explore_url,
)

query = Query(
explore=explore,
dimensions=(dimension,),
runtime=runtime,
query_id=query_id,
explore_url=explore_url,
)
assert query.to_profiler_format() == (
explore.name,
dimension.name,
runtime,
query_id,
explore_url,
)

0 comments on commit 5bdea9d

Please sign in to comment.