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.

Sign up for GitHub

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

Exclude explores #172

Merged
merged 15 commits into from
Apr 2, 2020
Merged

Exclude explores #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
de2ec46
Remove async methods from client class
joshtemple Feb 12, 2020
bfa87b1
Parametrize bad response checks
joshtemple Feb 14, 2020
8991d74
Add test to ensure error is set correctly
joshtemple Feb 17, 2020
3030609
Test that queries are cancelled
joshtemple Feb 17, 2020
638555f
Test case when queries are still running
joshtemple Feb 17, 2020
22a6c28
Fix assertion for cancellation
joshtemple Feb 19, 2020
cbcd805
add exclude CLI argument to parser
DylanBaker Mar 21, 2020
fba52f7
exclusion in validator functioning
DylanBaker Mar 21, 2020
2c74fc8
fix existing test_build_project test
DylanBaker Mar 21, 2020
52e9045
adding tests and removing empty models from project
DylanBaker Mar 21, 2020
231af45
more tests
DylanBaker Mar 21, 2020
711062a
fix duped tests:
DylanBaker Mar 29, 2020
7232598
Update spectacles/cli.py
DylanBaker Mar 31, 2020
46f8fb9
adding second explore to test models and adding ambiguous explore exc…
DylanBaker Mar 31, 2020
82c3fee
resolve conflicts
DylanBaker Apr 2, 2020
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
11 changes: 11 additions & 0 deletions spectacles/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ def main():
args.project,
args.branch,
args.explores,
args.exclude,
args.base_url,
args.client_id,
args.client_secret,
Expand Down Expand Up @@ -371,6 +372,15 @@ def _build_sql_subparser(
The '*' wildcard selects all models or explores. For instance,\
'model_name/*' would select all explores in the 'model_name' model.",
)
subparser.add_argument(
"--exclude",
nargs="+",
default=[],
help="Specify the explores spectacles should exclude when testing. \
List of selector strings in 'model_name/explore_name' format. \
The '*' wildcard excludes all models or explores. For instance,\
'model_name/*' would select all explores in the 'model_name' model.",
)
subparser.add_argument(
"--mode",
choices=["batch", "single", "hybrid"],
Expand Down Expand Up @@ -464,6 +474,7 @@ def run_sql(
project,
branch,
explores,
exclude,
base_url,
client_id,
client_secret,
Expand Down
1 change: 1 addition & 0 deletions spectacles/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ def create_query_task(self, query_id: int) -> str:
logger.debug("Starting query %d", query_id)
body = {"query_id": query_id, "result_format": "json_detail"}
url = utils.compose_url(self.api_url, path=["query_tasks"])

response = self.session.post(
url=url, json=body, params={"cache": "false"}, timeout=TIMEOUT_SEC
)
Expand Down
8 changes: 6 additions & 2 deletions spectacles/runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,14 @@ def __init__(

@log_duration
def validate_sql(
self, selectors: List[str], mode: str = "batch", concurrency: int = 10
self,
selectors: List[str],
exclusions: List[str],
mode: str = "batch",
concurrency: int = 10,
) -> Project:
sql_validator = SqlValidator(self.client, self.project, concurrency)
sql_validator.build_project(selectors)
sql_validator.build_project(selectors, exclusions)
project = sql_validator.validate(mode)
return project

Expand Down
31 changes: 28 additions & 3 deletions spectacles/validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def _select(self, choices: Sequence[str], select_from: Sequence) -> Sequence:
)
return [each for each in select_from if each.name in unique_choices]

def build_project(self, selectors: List[str]) -> None:
def build_project(self, selectors: List[str], exclusions: List[str]) -> None:
"""Creates an object representation of the project's LookML.

Args:
Expand All @@ -173,6 +173,7 @@ def build_project(self, selectors: List[str]) -> None:

"""
selection = self.parse_selectors(selectors)
exclusion = self.parse_selectors(exclusions)
logger.info(
f"Building LookML project hierarchy for project {self.project.name}"
)
Expand All @@ -194,9 +195,25 @@ def build_project(self, selectors: List[str]) -> None:
selected_models = self._select(
choices=tuple(selection.keys()), select_from=project_models
)
excluded_models = self._select(
choices=tuple(exclusion.keys()), select_from=project_models
)

for model in selected_models:
excluded_explores = {}
for model in excluded_models:
# Expand wildcard operator to include all specified or discovered explores
excluded_explore_names = exclusion[model.name]
if "*" in excluded_explore_names:
excluded_explore_names.remove("*")
excluded_explore_names.update(
set(explore.name for explore in model.explores)
)
Comment on lines +202 to +210
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there's a situation with model_A.explore_A and model_B.explore_A, and the --exclude argument is model_A.explore_A, would this still preserve explore_A in model_B? Any way we can add this case to the unit tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It will still preserve it. The explores to exclude are stored in lists against a key of the model name. We then only check against the explores within a given model: https://github.com/spectacles-ci/spectacles/pull/172/files#diff-cbee2ab0268f8f001868af4ca066ba04R228-R232

I'll definitely add some tests to confirm that though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joshtemple I've confirmed this is the case and have added an additional test test_build_project_one_ambiguous_explore_excluded to verify so.

I also actually re-factored some of the other tests to make the selection easier.


excluded_explores[model.name] = self._select(
choices=tuple(excluded_explore_names), select_from=model.explores
)

for model in selected_models:
selected_explore_names = selection[model.name]
if "*" in selected_explore_names:
selected_explore_names.remove("*")
Expand All @@ -207,6 +224,12 @@ def build_project(self, selectors: List[str]) -> None:
selected_explores = self._select(
choices=tuple(selected_explore_names), select_from=model.explores
)
if model.name in excluded_explores:
selected_explores = [
explore
for explore in selected_explores
if explore not in excluded_explores[model.name]
]

for explore in selected_explores:
dimensions_json = self.client.get_lookml_dimensions(
Expand All @@ -220,7 +243,9 @@ def build_project(self, selectors: List[str]) -> None:

model.explores = selected_explores

self.project.models = selected_models
self.project.models = [
model for model in selected_models if len(model.explores) > 0
]

def validate(self, mode: str = "batch") -> Project:
"""Queries selected explores and returns the project tree with errors."""
Expand Down
8 changes: 8 additions & 0 deletions tests/resources/response_models.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@
},
{
"explores": [
{
"description": null,
"label": "Test Explore One",
"hidden": false,
"group_label": "Test Explore One",
"name": "test_explore_one",
"can": {}
},
{
"description": null,
"label": "Test Explore Two",
Expand Down
110 changes: 106 additions & 4 deletions tests/test_sql_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def project():
),
]
explores_model_one = [Explore("test_explore_one", dimensions)]
explores_model_two = [Explore("test_explore_two", dimensions)]
explores_model_two = [
Explore("test_explore_one", dimensions),
Explore("test_explore_two", dimensions),
]
models = [
Model("test_model_one", "test_project", explores_model_one),
Model("test_model.two", "test_project", explores_model_two),
Expand Down Expand Up @@ -92,7 +95,106 @@ def test_parse_selectors_bad_format_raises_error():
def test_build_project(mock_get_models, mock_get_dimensions, project, validator):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(selectors=["*/*"])
validator.build_project(selectors=["*/*"], exclusions=[])
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_all_models_excluded(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(
selectors=["*/*"], exclusions=["test_model_one/*", "test_model.two/*"]
)
project.models = []
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_one_model_excluded(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(selectors=["*/*"], exclusions=["test_model_one/*"])
project.models = [
model for model in project.models if model.name != "test_model_one"
]
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_one_model_selected(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(selectors=["test_model.two/*"], exclusions=[])
project.models = [
model for model in project.models if model.name == "test_model.two"
]
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_one_explore_excluded(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(
selectors=["*/*"], exclusions=["test_model_one/test_explore_one"]
)
project.models = [
model for model in project.models if model.name != "test_model_one"
]
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_one_explore_selected(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(
selectors=["test_model.two/test_explore_two"], exclusions=[]
)
project.models = [
model for model in project.models if model.name == "test_model.two"
]
project.models[0].explores = [
explore
for explore in project.models[0].explores
if explore.name == "test_explore_two"
]
assert validator.project == project


@patch("spectacles.client.LookerClient.get_lookml_dimensions")
@patch("spectacles.client.LookerClient.get_lookml_models")
def test_build_project_one_ambiguous_explore_excluded(
mock_get_models, mock_get_dimensions, project, validator
):
mock_get_models.return_value = load("response_models.json")
mock_get_dimensions.return_value = load("response_dimensions.json")
validator.build_project(
selectors=["*/*"], exclusions=["test_model.two/test_explore_one"]
)
for model in project.models:
if model.name == "test_model.two":
model.explores = [
explore
for explore in model.explores
if explore.name != "test_explore_one"
]
assert validator.project == project


Expand Down Expand Up @@ -203,11 +305,11 @@ def test_handle_running_query(validator):

def test_count_explores(validator, project):
validator.project = project
assert validator._count_explores() == 2
assert validator._count_explores() == 3

explore = validator.project.models[0].explores[0]
validator.project.models[0].explores.extend([explore, explore])
assert validator._count_explores() == 4
assert validator._count_explores() == 5


def test_extract_error_details_error_dict(validator):
Expand Down