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

Add SQL validator support for measures #693

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
20 changes: 14 additions & 6 deletions spectacles/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ def main():
chunk_size=args.chunk_size,
pin_imports=pin_imports,
ignore_hidden=args.ignore_hidden,
ignore_measures=args.ignore_measures,
)
)
elif args.command == "assert":
Expand Down Expand Up @@ -597,7 +598,7 @@ def _build_sql_subparser(
"--fail-fast",
action="store_true",
help=(
"Test explore-by-explore instead of dimension-by-dimension. "
"Test explore-by-explore instead of field-by-field. "
"This means that validation takes less time but only returns the first "
"error identified in each explore. "
),
Expand Down Expand Up @@ -646,13 +647,18 @@ def _build_sql_subparser(
"--chunk-size",
type=int,
default=500,
help="Limit the size of explore-level queries by this number of dimensions.",
help="Limit the size of explore-level queries by this number of fields.",
)
subparser.add_argument(
"--ignore-hidden",
action="store_true",
help=("Exclude hidden fields from validation."),
)
subparser.add_argument(
"--ignore-measures",
action="store_true",
help=("Exclude measure fields from validation."),
)
_build_validator_subparser(subparser_action, subparser)
_build_select_subparser(subparser_action, subparser)

Expand Down Expand Up @@ -925,8 +931,9 @@ async def run_sql(
chunk_size,
pin_imports,
ignore_hidden,
ignore_measures,
) -> None:
"""Runs and validates the SQL for each selected LookML dimension."""
"""Runs and validates the SQL for each selected LookML field."""
# Don't trust env to ignore .netrc credentials
async_client = httpx.AsyncClient(trust_env=False)
try:
Expand All @@ -946,6 +953,7 @@ async def run_sql(
runtime_threshold,
chunk_size,
ignore_hidden,
ignore_measures,
)
finally:
await async_client.aclose()
Expand All @@ -956,7 +964,7 @@ async def run_sql(

errors = sorted(
results["errors"],
key=lambda x: (x["model"], x["explore"], x["metadata"].get("dimension")),
key=lambda x: (x["model"], x["explore"], x["metadata"].get("field")),
)

if errors:
Expand All @@ -967,13 +975,13 @@ async def run_sql(
message=error["message"],
sql=error["metadata"]["sql"],
log_dir=log_dir,
dimension=error["metadata"].get("dimension"),
field=error["metadata"].get("field"),
lookml_url=error["metadata"].get("lookml_url"),
)
if fail_fast:
logger.info(
printer.dim(
"\n\nTo determine the exact dimensions responsible for "
"\n\nTo determine the exact fields responsible for "
f"{'this error' if len(errors) == 1 else 'these errors'}, "
"you can rerun \nSpectacles without --fail-fast."
)
Expand Down
47 changes: 27 additions & 20 deletions spectacles/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,19 +635,21 @@ async def get_lookml_models(self, fields: Optional[List] = None) -> List[JsonDic
return response.json()

@backoff.on_exception(backoff.expo, BACKOFF_EXCEPTIONS, max_tries=DEFAULT_MAX_TRIES)
async def get_lookml_dimensions(self, model: str, explore: str) -> List[str]:
"""Gets all dimensions for an explore from the LookmlModel endpoint.
async def get_lookml_fields(
self, model: str, explore: str, ignore_measures: bool = False
) -> List[str]:
"""Gets all fields for an explore from the LookmlModel endpoint.

Args:
model: Name of LookML model to query.
explore: Name of LookML explore to query.

Returns:
List[str]: Names of all the dimensions in the specified explore. Dimension
names are returned in the format 'explore_name.dimension_name'.
List[str]: Names of all the fields in the specified explore. Field
names are returned in the format 'explore_name.field_name'.

"""
logger.debug(f"Getting all dimensions from explore {model}/{explore}")
logger.debug(f"Getting all fields from explore {model}/{explore}")
params = {"fields": ["fields"]}
url = utils.compose_url(
self.api_url,
Expand All @@ -659,31 +661,36 @@ async def get_lookml_dimensions(self, model: str, explore: str) -> List[str]:
response.raise_for_status()
except httpx.HTTPStatusError as error:
raise LookerApiError(
name="unable-to-get-dimension-lookml",
title="Couldn't retrieve dimensions.",
name="unable-to-get-field-lookml",
title="Couldn't retrieve LookML fields.",
status=response.status_code,
detail=(
"Unable to retrieve dimension LookML details "
"Unable to retrieve LookML field details "
f"for explore '{model}/{explore}'. Please try again."
),
response=response,
) from error

return response.json()["fields"]["dimensions"]
fields = response.json()["fields"]["dimensions"]

if not ignore_measures:
fields += response.json()["fields"]["measures"]

return fields

@backoff.on_exception(backoff.expo, BACKOFF_EXCEPTIONS, max_tries=5)
async def create_query(
self,
model: str,
explore: str,
dimensions: List[str],
fields: Optional[List] = None,
fields: List[str],
request_fields: Optional[List] = None,
) -> Dict:
"""Creates a Looker async query for one or more specified dimensions.
"""Creates a Looker async query for one or more specified fields.

The query created is a SELECT query, selecting all dimensions specified for a
The query created is a SELECT query, selecting all fields specified for a
certain model and explore. Looker builds the query using the `sql` field in the
LookML for each dimension.
LookML for each field.

If a Timeout exception is received, attempts to retry.

Expand All @@ -693,21 +700,21 @@ async def create_query(
"Creating async query for %s/%s/%s",
model,
explore,
"*" if len(dimensions) != 1 else dimensions[0],
"*" if len(fields) != 1 else fields[0],
)
body = {
"model": model,
"view": explore,
"fields": dimensions,
"fields": fields,
"limit": 0,
"filter_expression": "1=2",
}

params: Dict[str, list] = {}
if fields is None:
if request_fields is None:
params["fields"] = []
else:
params["fields"] = fields
params["fields"] = request_fields

url = utils.compose_url(self.api_url, path=["queries"], params=params)
response = await self.post(url=url, json=body, timeout=TIMEOUT_SEC)
Expand All @@ -720,7 +727,7 @@ async def create_query(
status=response.status_code,
detail=(
f"Failed to create query for {model}/{explore}/"
f'{"*" if len(dimensions) > 1 else dimensions[0]}. '
f'{"*" if len(fields) > 1 else fields[0]}. '
"Please try again."
),
response=response,
Expand All @@ -732,7 +739,7 @@ async def create_query(
"Query for %s/%s/%s created as query %s",
model,
explore,
"*" if len(dimensions) != 1 else dimensions[0],
"*" if len(fields) != 1 else fields[0],
query_id,
)
return result
Expand Down
6 changes: 3 additions & 3 deletions spectacles/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def __init__(
metadata = {
"line_number": line_number,
"lookml_url": lookml_url,
"dimension": field_name,
"field": field_name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noting that this is basically a breaking change as far as the API is concerned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we would need to release this with a corresponding change in the app/UI. It will likely need to account for both dimension and field, given old runs will have the old key.

"file_path": file_path,
"severity": severity,
}
Expand All @@ -134,15 +134,15 @@ def __init__(
self,
model: str,
explore: str,
dimension: Optional[str],
field: Optional[str],
sql: str,
message: str,
line_number: Optional[int] = None,
explore_url: Optional[str] = None,
lookml_url: Optional[str] = None,
):
metadata = {
"dimension": dimension,
"field": field,
"sql": sql,
"line_number": line_number,
"explore_url": explore_url,
Expand Down
8 changes: 4 additions & 4 deletions spectacles/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ def delete_color_codes(text: str) -> str:


def log_sql_error(
model: str, explore: str, sql: str, log_dir: str, dimension: Optional[str] = None
model: str, explore: str, sql: str, log_dir: str, field: Optional[str] = None
) -> Path:
file_name = (
model + "__" + explore + ("__" + dimension if dimension else "")
).replace(".", "_")
file_name = (model + "__" + explore + ("__" + field if field else "")).replace(
".", "_"
)
file_name += ".sql"
file_path = Path(log_dir) / "queries" / file_name

Expand Down
Loading