From 85986381dfb0f4e9ccd2221489224d3ddc2dcc16 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:34:58 +0000 Subject: [PATCH 01/34] add callback logging to backoff --- spectacles/client.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spectacles/client.py b/spectacles/client.py index 323f7bcc..eed5b4ad 100644 --- a/spectacles/client.py +++ b/spectacles/client.py @@ -66,17 +66,26 @@ def expired(self) -> bool: return False if time.time() < self.expires_at else True +def log_backoff(details: dict) -> None: + logger.debug( + f"Backing off {details['wait']:0.1f} seconds after {details['tries']} tries. " + f"Error: {details['exception'].__class__.__name__}" + ) + + def backoff_with_exceptions(func: Callable[..., Any]) -> Callable[..., Any]: @backoff.on_exception( backoff.expo, STATUS_EXCEPTIONS, giveup=giveup_unless_bad_gateway, max_tries=DEFAULT_RETRIES, + on_backoff=log_backoff, ) @backoff.on_exception( backoff.expo, NETWORK_EXCEPTIONS, max_tries=DEFAULT_NETWORK_RETRIES, + on_backoff=log_backoff, ) async def wrapper(*args: Any, **kwargs: Any) -> Any: if asyncio.iscoroutinefunction(func): From 14eb8f06bfca0820c25390fdf877f4156c147a52 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:42:18 +0000 Subject: [PATCH 02/34] skip lookml_models for incremental content validation --- spectacles/runner.py | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 161ec94c..2c7397f7 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -7,7 +7,7 @@ from spectacles.client import LOOKML_VALIDATION_TIMEOUT, LookerClient from spectacles.exceptions import LookerApiError, SpectaclesException, SqlError from spectacles.logger import GLOBAL_LOGGER as logger -from spectacles.lookml import CompiledSql, Explore, build_project +from spectacles.lookml import CompiledSql, Explore, build_project, Project from spectacles.models import JsonDict, SkipReason from spectacles.printer import print_header from spectacles.utils import time_hash @@ -543,22 +543,29 @@ async def validate_content( exclude_personal, folders, ) - logger.info( - "Building LookML project hierarchy for project " - f"'{self.project}' @ {self.branch_manager.ref}" - ) - project = await build_project( - self.client, - name=self.project, - filters=filters, - include_all_explores=True, - ) - explore_count = project.count_explores() - print_header( - f"Validating content based on {explore_count} " - f"{'explore' if explore_count == 1 else 'explores'}" - + (" [incremental mode] " if incremental else "") - ) + if not incremental: + logger.info( + "Building LookML project hierarchy for project " + f"'{self.project}' @ {self.branch_manager.ref}" + ) + project = await build_project( + self.client, + name=self.project, + filters=filters, + include_all_explores=True, + ) + explore_count = project.count_explores() + print_header( + f"Validating content based on {explore_count} " + f"{'explore' if explore_count == 1 else 'explores'}" + + (" [incremental mode] " if incremental else "") + ) + else: + logger.debug( + "Incremental mode is enabled, initializing and empty project" + ) + project = Project(name=self.project) + await validator.validate(project) results = project.get_results(validator="content", filters=filters) From 4813d2b590270e00558f40bdfb5ad141f5de5e2e Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:44:14 +0000 Subject: [PATCH 03/34] add empty models --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 2c7397f7..862886ef 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -564,7 +564,7 @@ async def validate_content( logger.debug( "Incremental mode is enabled, initializing and empty project" ) - project = Project(name=self.project) + project = Project(name=self.project, models=[]) await validator.validate(project) results = project.get_results(validator="content", filters=filters) From 52c2770329aede1f931f77017516dbc631739539 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:08:56 +0000 Subject: [PATCH 04/34] dont call lookml_models api for sql validator if explores are provided --- spectacles/lookml.py | 45 +++++++++++++++++++++++++++----------------- spectacles/runner.py | 4 ++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 3b40d8e6..4f8ae7ac 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -507,28 +507,39 @@ async def build_project( include_dimensions: bool = False, ignore_hidden_fields: bool = False, include_all_explores: bool = False, + get_full_project: bool = True, ) -> Project: """Creates an object (tree) representation of a LookML project.""" if filters is None: filters = ["*/*"] - models = [] - fields = ["name", "project_name", "explores"] - for lookmlmodel in await client.get_lookml_models(fields=fields): - model = Model.from_json(lookmlmodel) - if model.project_name == name: - models.append(model) - - if not models: - raise LookMlNotFound( - name="project-models-not-found", - title="No configured models found for the specified project.", - detail=( - f"Go to {client.base_url}/projects and confirm " - "a) at least one model exists for the project and " - "b) it has an active configuration." - ), - ) + if get_full_project: + models = [] + fields = ["name", "project_name", "explores"] + for lookmlmodel in await client.get_lookml_models(fields=fields): + model = Model.from_json(lookmlmodel) + if model.project_name == name: + models.append(model) + + if not models: + raise LookMlNotFound( + name="project-models-not-found", + title="No configured models found for the specified project.", + detail=( + f"Go to {client.base_url}/projects and confirm " + "a) at least one model exists for the project and " + "b) it has an active configuration." + ), + ) + else: + models = Dict[str, Model] + for filter in filters: + model, explore = filter.split("/") + if model not in models: + models[model] = Model(name=model, project_name=name, explores=[]) + if explore not in models[model].explores: + models[model].explores.append(Explore(name=explore, model_name=model)) + return Project(name=name, models=models.values()) # Prune to selected explores for non-content validators if not include_all_explores: diff --git a/spectacles/runner.py b/spectacles/runner.py index 862886ef..319b571a 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -355,6 +355,7 @@ async def validate_sql( ) -> JsonDict: if filters is None: filters = ["*/*"] + get_full_project = any("*" in f for f in filters) validator = SqlValidator(self.client, concurrency, runtime_threshold) ephemeral = True if incremental else None # Create explore-level tests for the desired ref @@ -367,6 +368,7 @@ async def validate_sql( filters=filters, include_dimensions=True, ignore_hidden_fields=ignore_hidden_fields, + get_full_project=get_full_project, ) base_explores: Set[CompiledSql] = set() if incremental: @@ -564,6 +566,8 @@ async def validate_content( logger.debug( "Incremental mode is enabled, initializing and empty project" ) + # Incremental content validation doesn't require knowing anything about the project hierarchy + # we can initialize and empty project and pass it to the validator project = Project(name=self.project, models=[]) await validator.validate(project) From 0df48918ad68f3b9d70b9e7ff0755d02044cb7e5 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:11:50 +0000 Subject: [PATCH 05/34] add some logging --- spectacles/lookml.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 4f8ae7ac..7e42f966 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -532,6 +532,8 @@ async def build_project( ), ) else: + # Create a project with only the models specified in the filters + logger.debug("Building project with only the filtered models") models = Dict[str, Model] for filter in filters: model, explore = filter.split("/") From 3dacc2f17a17ee47eb74184ecc1b512dac3f0655 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:16:04 +0000 Subject: [PATCH 06/34] fix dict initialization --- spectacles/lookml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 7e42f966..b0da2b3e 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -534,7 +534,7 @@ async def build_project( else: # Create a project with only the models specified in the filters logger.debug("Building project with only the filtered models") - models = Dict[str, Model] + models: Dict[str, Model] = {} for filter in filters: model, explore = filter.split("/") if model not in models: From d8b3fb138ef1c7b9dbe8c58b6b361c1ca4270943 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:24:58 +0000 Subject: [PATCH 07/34] fix models type and dont return --- spectacles/lookml.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index b0da2b3e..185db863 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -541,7 +541,8 @@ async def build_project( models[model] = Model(name=model, project_name=name, explores=[]) if explore not in models[model].explores: models[model].explores.append(Explore(name=explore, model_name=model)) - return Project(name=name, models=models.values()) + project = Project(name=name, models=models.values()) + models = project.models # Prune to selected explores for non-content validators if not include_all_explores: From 7b5127a2ad6423db75651e83cd5eea2fb7b3b831 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:52:12 +0000 Subject: [PATCH 08/34] dont filter for models/explores in content validation if the project isnt complete --- spectacles/lookml.py | 12 ++++++++ spectacles/runner.py | 52 +++++++++++++++----------------- spectacles/validators/content.py | 11 +++++-- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 185db863..db1c211e 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -307,6 +307,7 @@ class Project(LookMlObject): def __init__(self, name: str, models: Sequence[Model]) -> None: self.name = name self.models = models + self._is_complete = False def __eq__(self, other: Any) -> bool: if not isinstance(other, Project): @@ -344,6 +345,14 @@ def iter_dimensions(self, errored: bool = False) -> Iterable[Dimension]: else: yield dimension + @property + def is_complete_project(self) -> bool: + return self._is_complete + + @is_complete_project.setter + def is_complete_project(self, value: bool) -> None: + self._is_complete = value + @property def errored(self) -> Optional[bool]: if self.queried: @@ -531,6 +540,9 @@ async def build_project( "b) it has an active configuration." ), ) + else: + project.is_complete_project = True + else: # Create a project with only the models specified in the filters logger.debug("Building project with only the filtered models") diff --git a/spectacles/runner.py b/spectacles/runner.py index 319b571a..3879b3e0 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -355,7 +355,10 @@ async def validate_sql( ) -> JsonDict: if filters is None: filters = ["*/*"] - get_full_project = any("*" in f for f in filters) + else: + # Only build the full project from the API if we're using a wildcard filter and not in incremental mode + get_full_project = any("*" in f for f in filters) or incremental + validator = SqlValidator(self.client, concurrency, runtime_threshold) ephemeral = True if incremental else None # Create explore-level tests for the desired ref @@ -534,8 +537,10 @@ async def validate_content( exclude_personal: bool = False, folders: Optional[List[str]] = None, ) -> JsonDict: - if filters is None: - filters = ["*/*"] + if filters is not None: + # Only build the full project from the API if we're using a wildcard filter and not in incremental mode + get_full_project = any("*" in f for f in filters) or (not incremental) + if folders is None: folders = [] @@ -545,30 +550,23 @@ async def validate_content( exclude_personal, folders, ) - if not incremental: - logger.info( - "Building LookML project hierarchy for project " - f"'{self.project}' @ {self.branch_manager.ref}" - ) - project = await build_project( - self.client, - name=self.project, - filters=filters, - include_all_explores=True, - ) - explore_count = project.count_explores() - print_header( - f"Validating content based on {explore_count} " - f"{'explore' if explore_count == 1 else 'explores'}" - + (" [incremental mode] " if incremental else "") - ) - else: - logger.debug( - "Incremental mode is enabled, initializing and empty project" - ) - # Incremental content validation doesn't require knowing anything about the project hierarchy - # we can initialize and empty project and pass it to the validator - project = Project(name=self.project, models=[]) + logger.info( + "Building LookML project hierarchy for project " + f"'{self.project}' @ {self.branch_manager.ref}" + ) + project = await build_project( + self.client, + name=self.project, + filters=filters, + include_all_explores=True, + get_full_project=get_full_project, + ) + explore_count = project.count_explores() + print_header( + f"Validating content based on {explore_count} " + f"{'explore' if explore_count == 1 else 'explores'}" + + (" [incremental mode] " if incremental else "") + ) await validator.validate(project) results = project.get_results(validator="content", filters=filters) diff --git a/spectacles/validators/content.py b/spectacles/validators/content.py index 72c2c2ec..dfd4f0f4 100644 --- a/spectacles/validators/content.py +++ b/spectacles/validators/content.py @@ -133,7 +133,10 @@ def _get_tile_type(content: Dict[str, Any]) -> str: ) def _get_errors_from_result( - self, project: Project, result: Dict[str, Any], content_type: str + self, + project: Project, + result: Dict[str, Any], + content_type: str, ) -> List[ContentError]: content_errors: List[ContentError] = [] for error in result["errors"]: @@ -145,7 +148,11 @@ def _get_errors_from_result( else: explore = None # Skip errors that are not associated with selected explores or existing models - if explore or model: + if not project.is_complete_project: + logger.debug( + f"Project is not complete -- showing errors for all models/explores" + ) + if explore or model or not project.is_complete_project: content_id = result[content_type]["id"] folder = result[content_type].get("folder") folder_name: Optional[str] = folder.get("name") if folder else None From 385fd7c80d610479168747d311f3ba2b21b12773 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:55:23 +0000 Subject: [PATCH 09/34] adding log --- spectacles/runner.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spectacles/runner.py b/spectacles/runner.py index 3879b3e0..a92ed612 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -537,6 +537,9 @@ async def validate_content( exclude_personal: bool = False, folders: Optional[List[str]] = None, ) -> JsonDict: + logger.debug( + "Validating content", ref=ref, filters=filters, incremental=incremental + ) if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) From 8caccad4d099a1e0ae26263a5127a6a84b7d060d Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:55:45 +0000 Subject: [PATCH 10/34] another log --- spectacles/runner.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spectacles/runner.py b/spectacles/runner.py index a92ed612..d8282daf 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -543,6 +543,7 @@ async def validate_content( if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) + logger.debug(f"get_full_project = {get_full_project}") if folders is None: folders = [] From ceddf545259141eb8f62eccf6a3403f9480bbc6b Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:57:14 +0000 Subject: [PATCH 11/34] fix log --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d8282daf..5a47c12e 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -538,7 +538,7 @@ async def validate_content( folders: Optional[List[str]] = None, ) -> JsonDict: logger.debug( - "Validating content", ref=ref, filters=filters, incremental=incremental + f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode From 4c7069d0b2c5adc59a1f98af10f2a0483deacb55 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:00:09 +0000 Subject: [PATCH 12/34] check for default */* --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 5a47c12e..d5dde302 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -540,7 +540,7 @@ async def validate_content( logger.debug( f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) - if filters is not None: + if filters is not None or filters != ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) logger.debug(f"get_full_project = {get_full_project}") From 7d4b9730556e948e282800490eb30e4655f9bf68 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:01:39 +0000 Subject: [PATCH 13/34] fix conditional --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d5dde302..d468f326 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -540,7 +540,7 @@ async def validate_content( logger.debug( f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) - if filters is not None or filters != ["*/*"]: + if filters is not None or filters == ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) logger.debug(f"get_full_project = {get_full_project}") From c7f0417de57ef91cb813566aeb21e3e5b4b897fe Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:03:21 +0000 Subject: [PATCH 14/34] ignore if */* --- spectacles/runner.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d468f326..c48c7969 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -542,7 +542,9 @@ async def validate_content( ) if filters is not None or filters == ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode - get_full_project = any("*" in f for f in filters) or (not incremental) + get_full_project = any("*" in f for f in filters if f != "*/*") or ( + not incremental + ) logger.debug(f"get_full_project = {get_full_project}") if folders is None: From 18da8ffaddd77c9b378cf28c59deecd0624bfb08 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:04:58 +0000 Subject: [PATCH 15/34] set is_complete_project better --- spectacles/lookml.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index db1c211e..dc9e9d92 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -521,6 +521,7 @@ async def build_project( """Creates an object (tree) representation of a LookML project.""" if filters is None: filters = ["*/*"] + is_complete_project = False if get_full_project: models = [] @@ -540,8 +541,7 @@ async def build_project( "b) it has an active configuration." ), ) - else: - project.is_complete_project = True + is_complete_project = True else: # Create a project with only the models specified in the filters @@ -581,4 +581,6 @@ async def build_project( else: project = Project(name, [m for m in models if len(m.explores) > 0]) + # Indicates whether the project has all of the models/explores or just the selected ones + project.is_complete_project = is_complete_project return project From 881d184378f71c8e34239518ad522f9d11198962 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Mon, 2 Dec 2024 18:29:58 +0000 Subject: [PATCH 16/34] revert content changes --- spectacles/runner.py | 2 -- spectacles/validators/content.py | 11 ++--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index c48c7969..951044ff 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -565,7 +565,6 @@ async def validate_content( name=self.project, filters=filters, include_all_explores=True, - get_full_project=get_full_project, ) explore_count = project.count_explores() print_header( @@ -573,7 +572,6 @@ async def validate_content( f"{'explore' if explore_count == 1 else 'explores'}" + (" [incremental mode] " if incremental else "") ) - await validator.validate(project) results = project.get_results(validator="content", filters=filters) diff --git a/spectacles/validators/content.py b/spectacles/validators/content.py index dfd4f0f4..72c2c2ec 100644 --- a/spectacles/validators/content.py +++ b/spectacles/validators/content.py @@ -133,10 +133,7 @@ def _get_tile_type(content: Dict[str, Any]) -> str: ) def _get_errors_from_result( - self, - project: Project, - result: Dict[str, Any], - content_type: str, + self, project: Project, result: Dict[str, Any], content_type: str ) -> List[ContentError]: content_errors: List[ContentError] = [] for error in result["errors"]: @@ -148,11 +145,7 @@ def _get_errors_from_result( else: explore = None # Skip errors that are not associated with selected explores or existing models - if not project.is_complete_project: - logger.debug( - f"Project is not complete -- showing errors for all models/explores" - ) - if explore or model or not project.is_complete_project: + if explore or model: content_id = result[content_type]["id"] folder = result[content_type].get("folder") folder_name: Optional[str] = folder.get("name") if folder else None From 9c6d31e5a3100ad47560d74965daabc73002a518 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:34:58 +0000 Subject: [PATCH 17/34] add callback logging to backoff --- spectacles/client.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spectacles/client.py b/spectacles/client.py index 323f7bcc..eed5b4ad 100644 --- a/spectacles/client.py +++ b/spectacles/client.py @@ -66,17 +66,26 @@ def expired(self) -> bool: return False if time.time() < self.expires_at else True +def log_backoff(details: dict) -> None: + logger.debug( + f"Backing off {details['wait']:0.1f} seconds after {details['tries']} tries. " + f"Error: {details['exception'].__class__.__name__}" + ) + + def backoff_with_exceptions(func: Callable[..., Any]) -> Callable[..., Any]: @backoff.on_exception( backoff.expo, STATUS_EXCEPTIONS, giveup=giveup_unless_bad_gateway, max_tries=DEFAULT_RETRIES, + on_backoff=log_backoff, ) @backoff.on_exception( backoff.expo, NETWORK_EXCEPTIONS, max_tries=DEFAULT_NETWORK_RETRIES, + on_backoff=log_backoff, ) async def wrapper(*args: Any, **kwargs: Any) -> Any: if asyncio.iscoroutinefunction(func): From 15b9873d8d428c3ace0d96d142ed7d32ca8e6846 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:42:18 +0000 Subject: [PATCH 18/34] skip lookml_models for incremental content validation --- spectacles/runner.py | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 161ec94c..2c7397f7 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -7,7 +7,7 @@ from spectacles.client import LOOKML_VALIDATION_TIMEOUT, LookerClient from spectacles.exceptions import LookerApiError, SpectaclesException, SqlError from spectacles.logger import GLOBAL_LOGGER as logger -from spectacles.lookml import CompiledSql, Explore, build_project +from spectacles.lookml import CompiledSql, Explore, build_project, Project from spectacles.models import JsonDict, SkipReason from spectacles.printer import print_header from spectacles.utils import time_hash @@ -543,22 +543,29 @@ async def validate_content( exclude_personal, folders, ) - logger.info( - "Building LookML project hierarchy for project " - f"'{self.project}' @ {self.branch_manager.ref}" - ) - project = await build_project( - self.client, - name=self.project, - filters=filters, - include_all_explores=True, - ) - explore_count = project.count_explores() - print_header( - f"Validating content based on {explore_count} " - f"{'explore' if explore_count == 1 else 'explores'}" - + (" [incremental mode] " if incremental else "") - ) + if not incremental: + logger.info( + "Building LookML project hierarchy for project " + f"'{self.project}' @ {self.branch_manager.ref}" + ) + project = await build_project( + self.client, + name=self.project, + filters=filters, + include_all_explores=True, + ) + explore_count = project.count_explores() + print_header( + f"Validating content based on {explore_count} " + f"{'explore' if explore_count == 1 else 'explores'}" + + (" [incremental mode] " if incremental else "") + ) + else: + logger.debug( + "Incremental mode is enabled, initializing and empty project" + ) + project = Project(name=self.project) + await validator.validate(project) results = project.get_results(validator="content", filters=filters) From d7804ee56ccd58e17c50136de88e9145a1098f4c Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 17:44:14 +0000 Subject: [PATCH 19/34] add empty models --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 2c7397f7..862886ef 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -564,7 +564,7 @@ async def validate_content( logger.debug( "Incremental mode is enabled, initializing and empty project" ) - project = Project(name=self.project) + project = Project(name=self.project, models=[]) await validator.validate(project) results = project.get_results(validator="content", filters=filters) From 237e4f40419c9b3a6677c977c32c4fadb796cb5c Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:08:56 +0000 Subject: [PATCH 20/34] dont call lookml_models api for sql validator if explores are provided --- spectacles/lookml.py | 45 +++++++++++++++++++++++++++----------------- spectacles/runner.py | 4 ++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 3b40d8e6..4f8ae7ac 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -507,28 +507,39 @@ async def build_project( include_dimensions: bool = False, ignore_hidden_fields: bool = False, include_all_explores: bool = False, + get_full_project: bool = True, ) -> Project: """Creates an object (tree) representation of a LookML project.""" if filters is None: filters = ["*/*"] - models = [] - fields = ["name", "project_name", "explores"] - for lookmlmodel in await client.get_lookml_models(fields=fields): - model = Model.from_json(lookmlmodel) - if model.project_name == name: - models.append(model) - - if not models: - raise LookMlNotFound( - name="project-models-not-found", - title="No configured models found for the specified project.", - detail=( - f"Go to {client.base_url}/projects and confirm " - "a) at least one model exists for the project and " - "b) it has an active configuration." - ), - ) + if get_full_project: + models = [] + fields = ["name", "project_name", "explores"] + for lookmlmodel in await client.get_lookml_models(fields=fields): + model = Model.from_json(lookmlmodel) + if model.project_name == name: + models.append(model) + + if not models: + raise LookMlNotFound( + name="project-models-not-found", + title="No configured models found for the specified project.", + detail=( + f"Go to {client.base_url}/projects and confirm " + "a) at least one model exists for the project and " + "b) it has an active configuration." + ), + ) + else: + models = Dict[str, Model] + for filter in filters: + model, explore = filter.split("/") + if model not in models: + models[model] = Model(name=model, project_name=name, explores=[]) + if explore not in models[model].explores: + models[model].explores.append(Explore(name=explore, model_name=model)) + return Project(name=name, models=models.values()) # Prune to selected explores for non-content validators if not include_all_explores: diff --git a/spectacles/runner.py b/spectacles/runner.py index 862886ef..319b571a 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -355,6 +355,7 @@ async def validate_sql( ) -> JsonDict: if filters is None: filters = ["*/*"] + get_full_project = any("*" in f for f in filters) validator = SqlValidator(self.client, concurrency, runtime_threshold) ephemeral = True if incremental else None # Create explore-level tests for the desired ref @@ -367,6 +368,7 @@ async def validate_sql( filters=filters, include_dimensions=True, ignore_hidden_fields=ignore_hidden_fields, + get_full_project=get_full_project, ) base_explores: Set[CompiledSql] = set() if incremental: @@ -564,6 +566,8 @@ async def validate_content( logger.debug( "Incremental mode is enabled, initializing and empty project" ) + # Incremental content validation doesn't require knowing anything about the project hierarchy + # we can initialize and empty project and pass it to the validator project = Project(name=self.project, models=[]) await validator.validate(project) From 882af7a21bc672a99a92105412ae7086052ff67f Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:11:50 +0000 Subject: [PATCH 21/34] add some logging --- spectacles/lookml.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 4f8ae7ac..7e42f966 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -532,6 +532,8 @@ async def build_project( ), ) else: + # Create a project with only the models specified in the filters + logger.debug("Building project with only the filtered models") models = Dict[str, Model] for filter in filters: model, explore = filter.split("/") From a9593b9a844d1128a08ad4b1ad0565d134852cb9 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:16:04 +0000 Subject: [PATCH 22/34] fix dict initialization --- spectacles/lookml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 7e42f966..b0da2b3e 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -534,7 +534,7 @@ async def build_project( else: # Create a project with only the models specified in the filters logger.debug("Building project with only the filtered models") - models = Dict[str, Model] + models: Dict[str, Model] = {} for filter in filters: model, explore = filter.split("/") if model not in models: From dd34557f534d3c5a7cb66d3c76abad18652dffa8 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:24:58 +0000 Subject: [PATCH 23/34] fix models type and dont return --- spectacles/lookml.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index b0da2b3e..185db863 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -541,7 +541,8 @@ async def build_project( models[model] = Model(name=model, project_name=name, explores=[]) if explore not in models[model].explores: models[model].explores.append(Explore(name=explore, model_name=model)) - return Project(name=name, models=models.values()) + project = Project(name=name, models=models.values()) + models = project.models # Prune to selected explores for non-content validators if not include_all_explores: From 2caa8d3300aaed9622dc783c64fe0da3f950549c Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:52:12 +0000 Subject: [PATCH 24/34] dont filter for models/explores in content validation if the project isnt complete --- spectacles/lookml.py | 12 ++++++++ spectacles/runner.py | 52 +++++++++++++++----------------- spectacles/validators/content.py | 11 +++++-- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index 185db863..db1c211e 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -307,6 +307,7 @@ class Project(LookMlObject): def __init__(self, name: str, models: Sequence[Model]) -> None: self.name = name self.models = models + self._is_complete = False def __eq__(self, other: Any) -> bool: if not isinstance(other, Project): @@ -344,6 +345,14 @@ def iter_dimensions(self, errored: bool = False) -> Iterable[Dimension]: else: yield dimension + @property + def is_complete_project(self) -> bool: + return self._is_complete + + @is_complete_project.setter + def is_complete_project(self, value: bool) -> None: + self._is_complete = value + @property def errored(self) -> Optional[bool]: if self.queried: @@ -531,6 +540,9 @@ async def build_project( "b) it has an active configuration." ), ) + else: + project.is_complete_project = True + else: # Create a project with only the models specified in the filters logger.debug("Building project with only the filtered models") diff --git a/spectacles/runner.py b/spectacles/runner.py index 319b571a..3879b3e0 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -355,7 +355,10 @@ async def validate_sql( ) -> JsonDict: if filters is None: filters = ["*/*"] - get_full_project = any("*" in f for f in filters) + else: + # Only build the full project from the API if we're using a wildcard filter and not in incremental mode + get_full_project = any("*" in f for f in filters) or incremental + validator = SqlValidator(self.client, concurrency, runtime_threshold) ephemeral = True if incremental else None # Create explore-level tests for the desired ref @@ -534,8 +537,10 @@ async def validate_content( exclude_personal: bool = False, folders: Optional[List[str]] = None, ) -> JsonDict: - if filters is None: - filters = ["*/*"] + if filters is not None: + # Only build the full project from the API if we're using a wildcard filter and not in incremental mode + get_full_project = any("*" in f for f in filters) or (not incremental) + if folders is None: folders = [] @@ -545,30 +550,23 @@ async def validate_content( exclude_personal, folders, ) - if not incremental: - logger.info( - "Building LookML project hierarchy for project " - f"'{self.project}' @ {self.branch_manager.ref}" - ) - project = await build_project( - self.client, - name=self.project, - filters=filters, - include_all_explores=True, - ) - explore_count = project.count_explores() - print_header( - f"Validating content based on {explore_count} " - f"{'explore' if explore_count == 1 else 'explores'}" - + (" [incremental mode] " if incremental else "") - ) - else: - logger.debug( - "Incremental mode is enabled, initializing and empty project" - ) - # Incremental content validation doesn't require knowing anything about the project hierarchy - # we can initialize and empty project and pass it to the validator - project = Project(name=self.project, models=[]) + logger.info( + "Building LookML project hierarchy for project " + f"'{self.project}' @ {self.branch_manager.ref}" + ) + project = await build_project( + self.client, + name=self.project, + filters=filters, + include_all_explores=True, + get_full_project=get_full_project, + ) + explore_count = project.count_explores() + print_header( + f"Validating content based on {explore_count} " + f"{'explore' if explore_count == 1 else 'explores'}" + + (" [incremental mode] " if incremental else "") + ) await validator.validate(project) results = project.get_results(validator="content", filters=filters) diff --git a/spectacles/validators/content.py b/spectacles/validators/content.py index 72c2c2ec..dfd4f0f4 100644 --- a/spectacles/validators/content.py +++ b/spectacles/validators/content.py @@ -133,7 +133,10 @@ def _get_tile_type(content: Dict[str, Any]) -> str: ) def _get_errors_from_result( - self, project: Project, result: Dict[str, Any], content_type: str + self, + project: Project, + result: Dict[str, Any], + content_type: str, ) -> List[ContentError]: content_errors: List[ContentError] = [] for error in result["errors"]: @@ -145,7 +148,11 @@ def _get_errors_from_result( else: explore = None # Skip errors that are not associated with selected explores or existing models - if explore or model: + if not project.is_complete_project: + logger.debug( + f"Project is not complete -- showing errors for all models/explores" + ) + if explore or model or not project.is_complete_project: content_id = result[content_type]["id"] folder = result[content_type].get("folder") folder_name: Optional[str] = folder.get("name") if folder else None From e7b6271a5b56f9a96491ddff587bfadbca342d3d Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:55:23 +0000 Subject: [PATCH 25/34] adding log --- spectacles/runner.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spectacles/runner.py b/spectacles/runner.py index 3879b3e0..a92ed612 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -537,6 +537,9 @@ async def validate_content( exclude_personal: bool = False, folders: Optional[List[str]] = None, ) -> JsonDict: + logger.debug( + "Validating content", ref=ref, filters=filters, incremental=incremental + ) if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) From 0c238af7d6df6058f211d387e4fe300de6acaae6 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:55:45 +0000 Subject: [PATCH 26/34] another log --- spectacles/runner.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spectacles/runner.py b/spectacles/runner.py index a92ed612..d8282daf 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -543,6 +543,7 @@ async def validate_content( if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) + logger.debug(f"get_full_project = {get_full_project}") if folders is None: folders = [] From a032a5c57680f31952591b9a0482ef2662b9704c Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 18:57:14 +0000 Subject: [PATCH 27/34] fix log --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d8282daf..5a47c12e 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -538,7 +538,7 @@ async def validate_content( folders: Optional[List[str]] = None, ) -> JsonDict: logger.debug( - "Validating content", ref=ref, filters=filters, incremental=incremental + f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) if filters is not None: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode From ba259345ebe16ee5f55093a5f5a95ad18620aada Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:00:09 +0000 Subject: [PATCH 28/34] check for default */* --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index 5a47c12e..d5dde302 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -540,7 +540,7 @@ async def validate_content( logger.debug( f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) - if filters is not None: + if filters is not None or filters != ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) logger.debug(f"get_full_project = {get_full_project}") From 4688e7b44f6b96882d19af6f33155ccc3bc8e6cc Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:01:39 +0000 Subject: [PATCH 29/34] fix conditional --- spectacles/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d5dde302..d468f326 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -540,7 +540,7 @@ async def validate_content( logger.debug( f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", ) - if filters is not None or filters != ["*/*"]: + if filters is not None or filters == ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode get_full_project = any("*" in f for f in filters) or (not incremental) logger.debug(f"get_full_project = {get_full_project}") From cb484e8395204de951e5e5a019cc273fd2444972 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:03:21 +0000 Subject: [PATCH 30/34] ignore if */* --- spectacles/runner.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index d468f326..c48c7969 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -542,7 +542,9 @@ async def validate_content( ) if filters is not None or filters == ["*/*"]: # Only build the full project from the API if we're using a wildcard filter and not in incremental mode - get_full_project = any("*" in f for f in filters) or (not incremental) + get_full_project = any("*" in f for f in filters if f != "*/*") or ( + not incremental + ) logger.debug(f"get_full_project = {get_full_project}") if folders is None: From a49c7e89d68f562e2447f7d3b4fcbe132d3ddedd Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 27 Nov 2024 19:04:58 +0000 Subject: [PATCH 31/34] set is_complete_project better --- spectacles/lookml.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spectacles/lookml.py b/spectacles/lookml.py index db1c211e..dc9e9d92 100644 --- a/spectacles/lookml.py +++ b/spectacles/lookml.py @@ -521,6 +521,7 @@ async def build_project( """Creates an object (tree) representation of a LookML project.""" if filters is None: filters = ["*/*"] + is_complete_project = False if get_full_project: models = [] @@ -540,8 +541,7 @@ async def build_project( "b) it has an active configuration." ), ) - else: - project.is_complete_project = True + is_complete_project = True else: # Create a project with only the models specified in the filters @@ -581,4 +581,6 @@ async def build_project( else: project = Project(name, [m for m in models if len(m.explores) > 0]) + # Indicates whether the project has all of the models/explores or just the selected ones + project.is_complete_project = is_complete_project return project From f5c5b3c3335ef3b092abb7b86569ab3642dd757e Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Mon, 2 Dec 2024 18:29:58 +0000 Subject: [PATCH 32/34] revert content changes --- spectacles/runner.py | 2 -- spectacles/validators/content.py | 11 ++--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/spectacles/runner.py b/spectacles/runner.py index c48c7969..951044ff 100644 --- a/spectacles/runner.py +++ b/spectacles/runner.py @@ -565,7 +565,6 @@ async def validate_content( name=self.project, filters=filters, include_all_explores=True, - get_full_project=get_full_project, ) explore_count = project.count_explores() print_header( @@ -573,7 +572,6 @@ async def validate_content( f"{'explore' if explore_count == 1 else 'explores'}" + (" [incremental mode] " if incremental else "") ) - await validator.validate(project) results = project.get_results(validator="content", filters=filters) diff --git a/spectacles/validators/content.py b/spectacles/validators/content.py index dfd4f0f4..72c2c2ec 100644 --- a/spectacles/validators/content.py +++ b/spectacles/validators/content.py @@ -133,10 +133,7 @@ def _get_tile_type(content: Dict[str, Any]) -> str: ) def _get_errors_from_result( - self, - project: Project, - result: Dict[str, Any], - content_type: str, + self, project: Project, result: Dict[str, Any], content_type: str ) -> List[ContentError]: content_errors: List[ContentError] = [] for error in result["errors"]: @@ -148,11 +145,7 @@ def _get_errors_from_result( else: explore = None # Skip errors that are not associated with selected explores or existing models - if not project.is_complete_project: - logger.debug( - f"Project is not complete -- showing errors for all models/explores" - ) - if explore or model or not project.is_complete_project: + if explore or model: content_id = result[content_type]["id"] folder = result[content_type].get("folder") folder_name: Optional[str] = folder.get("name") if folder else None From 73e1db6ac93abd746f6c28fc989353ef077936cb Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 4 Dec 2024 15:10:07 +0000 Subject: [PATCH 33/34] add logging for error --- spectacles/client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/spectacles/client.py b/spectacles/client.py index eed5b4ad..ea84794e 100644 --- a/spectacles/client.py +++ b/spectacles/client.py @@ -717,6 +717,7 @@ async def get_lookml_dimensions( try: response.raise_for_status() except httpx.HTTPStatusError as error: + logger.debug(f"Error response: {response.text}") raise LookerApiError( name="unable-to-get-dimension-lookml", title="Couldn't retrieve dimensions.", From 3197eac94c4686680aeffa5d25b752a863e41407 Mon Sep 17 00:00:00 2001 From: Jason Brownstein Date: Wed, 4 Dec 2024 15:13:17 +0000 Subject: [PATCH 34/34] make it an info log --- spectacles/client.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spectacles/client.py b/spectacles/client.py index ea84794e..06149e26 100644 --- a/spectacles/client.py +++ b/spectacles/client.py @@ -717,7 +717,9 @@ async def get_lookml_dimensions( try: response.raise_for_status() except httpx.HTTPStatusError as error: - logger.debug(f"Error response: {response.text}") + logger.info( + f"Error code: {response.status_code} for url {url} response: {response.text}" + ) raise LookerApiError( name="unable-to-get-dimension-lookml", title="Couldn't retrieve dimensions.",