-
Notifications
You must be signed in to change notification settings - Fork 35
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
[Work in progress] Refactor to only run get_all_models
if necessary
#828
Closed
jsnb-devoted
wants to merge
23
commits into
spectacles-ci:master
from
jsnb-devoted:devoted-health-build-project
Closed
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
8598638
add callback logging to backoff
jsnb-devoted 14eb8f0
skip lookml_models for incremental content validation
jsnb-devoted 4813d2b
add empty models
jsnb-devoted 52c2770
dont call lookml_models api for sql validator if explores are provided
jsnb-devoted 0df4891
add some logging
jsnb-devoted 3dacc2f
fix dict initialization
jsnb-devoted d8b3fb1
fix models type and dont return
jsnb-devoted 7b5127a
dont filter for models/explores in content validation if the project …
jsnb-devoted 385fd7c
adding log
jsnb-devoted 8caccad
another log
jsnb-devoted ceddf54
fix log
jsnb-devoted 4c7069d
check for default */*
jsnb-devoted 7d4b973
fix conditional
jsnb-devoted c7f0417
ignore if */*
jsnb-devoted 18da8ff
set is_complete_project better
jsnb-devoted 7123d47
append to content errors no matter what
jsnb-devoted aaaef2e
create models/explores if they dont exist
jsnb-devoted f52d182
make a list
jsnb-devoted 51f470a
init variable reference
jsnb-devoted 4bc86a7
remove noisy log
jsnb-devoted b6bb970
fix header printing
jsnb-devoted b5054e1
also get partial project for the target
jsnb-devoted 1193f49
fix comment
jsnb-devoted File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -355,6 +355,10 @@ async def validate_sql( | |
) -> JsonDict: | ||
if filters is None: | ||
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 | ||
|
@@ -367,6 +371,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: | ||
|
@@ -532,8 +537,16 @@ async def validate_content( | |
exclude_personal: bool = False, | ||
folders: Optional[List[str]] = None, | ||
) -> JsonDict: | ||
if filters is None: | ||
filters = ["*/*"] | ||
logger.debug( | ||
f"Validating content. ref={ref}, filters={filters}, incremental={incremental}", | ||
) | ||
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 if f != "*/*") or ( | ||
not incremental | ||
) | ||
logger.debug(f"get_full_project = {get_full_project}") | ||
|
||
if folders is None: | ||
folders = [] | ||
|
||
|
@@ -552,13 +565,21 @@ async def validate_content( | |
name=self.project, | ||
filters=filters, | ||
include_all_explores=True, | ||
get_full_project=get_full_project, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does |
||
) | ||
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 project.is_complete_project: | ||
print_header( | ||
f"Validating content based on all explores " | ||
+ (" [incremental mode] " if incremental else "") | ||
) | ||
else: | ||
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) | ||
|
||
|
@@ -574,6 +595,7 @@ async def validate_content( | |
name=self.project, | ||
filters=filters, | ||
include_all_explores=True, | ||
get_full_project=get_full_project, | ||
) | ||
await validator.validate(target_project) | ||
target_results = target_project.get_results( | ||
|
@@ -586,7 +608,7 @@ async def validate_content( | |
|
||
@staticmethod | ||
def _incremental_results(base: JsonDict, target: JsonDict) -> JsonDict: | ||
"""Returns a new result with only the additional errors in `additional`.""" | ||
"""Returns a new result with only the additional errors in `target`.""" | ||
diff: JsonDict = { | ||
"validator": "content", | ||
# Start with models and explores we know passed for the base ref | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is notionally a world where you have manually passed in every explore within a project and this is true, even though
get_full_project
was passed as false. This may not matter, but wanted to flag it.