Removing Materializations #445
Conversation
There was a problem hiding this comment.
Thank you for being so thorough! This looks great 🙂 after reading through the changes I pull down the branch and double checked for any references to materializations, and the only thing I found was a mention in the change log (and that seems appropriate to stay). Can't wait to get this merged as I think removing materializations moves the cutover to dbt-semantic-interfaces forward dramatically (a bunch of the tangled code was in materializations).
| @@ -1,43 +0,0 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
We shouldn't be removing this file's tests wholesale. I think we should instead modify the test_can_configure_model_validator_rules to continue working and test_cant_configure_model_validator_without_rules needs no changes.
There was a problem hiding this comment.
Made changes based on your suggestions, let me know if that resolves the issue!
| model = model_with_materialization( | ||
| simple_model__with_primary_transforms, | ||
| [ | ||
| materialization_with_guaranteed_meta( | ||
| name="foobar", | ||
| metrics=["invalid_bookings"], | ||
| dimensions=[DataSet.metric_time_dimension_name()], | ||
| ) | ||
| ], | ||
| ) | ||
|
|
||
| # confirm that with the default configuration, an issue is raised | ||
| issues = ModelValidator().validate_model(model).issues | ||
| assert len(issues.all_issues) == 1, f"ModelValidator with default rules had unexpected number of issues {issues}" | ||
|
|
||
| # confirm that a custom configuration excluding ValidMaterializationRule, no issue is raised | ||
| rules = [rule for rule in ModelValidator.DEFAULT_RULES if rule.__class__ is not ValidMaterializationRule] |
There was a problem hiding this comment.
I think this can be changed to the following
model = copy.deepcopy(simple_model__with_primary_transforms)
model.metrics.append(metric_with_guaranteed_meta(
name='metric_doesnt_exist_squared',
type=MetricType.DERIVED,
type_params=MetricTypeParams(
expr='metric_doesnt_exist * metric_doesnt_exist',
metrics=[MetricInput(name='metric_doesnt_exist')]
),
))
# confirm that with the default configuration, an issue is raised
issues = ModelValidator().validate_model(model).issues
assert len(issues.all_issues) == 1, f"ModelValidator with default rules had unexpected number of issues {issues}"
# confirm that a custom configuration excluding ValidMaterializationRule, no issue is raised
rules = [rule for rule in ModelValidator.DEFAULT_RULES if rule.__class__ is not DerivedMetricRule]
Materializations
This PR removes materializations. Originally it was done in three phases:
Unfortunately I ran into some issue with graphite because I was working on a forked version of metricflow. When trying to get it back onto this repo I ran into some real issues and just decided to fall back onto the old PR. In the future though, I've got graphite all set up and should be good to go for review and stacked commits!