Skip to content

tags and owners sidebars#452

Merged
IDoneShaveIt merged 17 commits intomasterfrom
tags-and-owners-sidebars
Dec 13, 2022
Merged

tags and owners sidebars#452
IDoneShaveIt merged 17 commits intomasterfrom
tags-and-owners-sidebars

Conversation

@IDoneShaveIt
Copy link
Contributor

Added 2 more sidebars to the report - tags & owners.

@IDoneShaveIt IDoneShaveIt requested a review from elongl December 4, 2022 13:14
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

👋 @IDoneShaveIt
Thank you for raising your pull request.
Please make sure you have documented all user-facing changes in the docs.
You can do this by editing the docs files in this pull request.

Comment on lines +23 to +31
test_tuns_totals: Dict[str, TotalsSchema],
models: Dict[str, NormalizedModelSchema],
sources: Dict[str, NormalizedSourceSchema],
models_runs: List[ModelRunsSchema],
) -> FiltersSchema:
test_results_filters = self._get_test_filters(
test_results_totals, models, sources
)
test_runs_filters = self._get_test_filters(test_tuns_totals, models, sources)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
test_tuns_totals: Dict[str, TotalsSchema],
models: Dict[str, NormalizedModelSchema],
sources: Dict[str, NormalizedSourceSchema],
models_runs: List[ModelRunsSchema],
) -> FiltersSchema:
test_results_filters = self._get_test_filters(
test_results_totals, models, sources
)
test_runs_filters = self._get_test_filters(test_tuns_totals, models, sources)
test_runs_totals: Dict[str, TotalsSchema],
models: Dict[str, NormalizedModelSchema],
sources: Dict[str, NormalizedSourceSchema],
models_runs: List[ModelRunsSchema],
) -> FiltersSchema:
test_results_filters = self._get_test_filters(
test_results_totals, models, sources
)
test_runs_filters = self._get_test_filters(test_runs_totals, models, sources)

and not total.errors
and not total.passed
):
no_tests_filter.add_model_unique_id(model_unique_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this happen? or just for safety?
(e.g. can totals contain a key that doesn't have tests?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes :)
You can define models without tests / run dbt test -s which will cause only partial tests to run.
On both cases we can have a situation where there are no tests 🙂



class InvocationSchema(BaseModel):
affected_rows: Optional[int]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need both TotalsInvocationsSchema and TotalsSchema?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well they used to be different and I somehow like that we define a schema for each data type (even if they are the same).
But I agree that currently its a bit redundant

@IDoneShaveIt IDoneShaveIt merged commit 88178b6 into master Dec 13, 2022
@IDoneShaveIt IDoneShaveIt deleted the tags-and-owners-sidebars branch December 13, 2022 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants