Don't lose materialization interval tracking when re-applying feature views #1559
Conversation
|
/kind bug |
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
+ Coverage 77.84% 83.70% +5.86%
==========================================
Files 66 67 +1
Lines 5688 5850 +162
==========================================
+ Hits 4428 4897 +469
+ Misses 1260 953 -307
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -185,11 +185,60 @@ def updater(registry_proto: RegistryProto): | |||
| == feature_view_proto.spec.name | |||
| and existing_feature_view_proto.spec.project == project | |||
| ): | |||
| # do not update if feature view has not changed; updating will erase tracked materialization intervals | |||
There was a problem hiding this comment.
It's not immediately clear to me what is happening here. Mind explaining?
There was a problem hiding this comment.
Yeah, this function apply_feature_view does a "destructive update" - it erases tracked materialization intervals so as far as Feast is aware, no time interval of this feature view has been materialized yet. This is what we want if we changed the feature view, but if we didn't it'll cause unnecessary rematerializations. So this part just says don't do a destructive update if the feature view is exactly the same as what's already in the registry
jklegar
force-pushed
the
jacob_fix_materialization_tracking
branch
from
73e7c41 to
c1db634
Compare
… views Signed-off-by: Jacob Klegar <[email protected]>
Signed-off-by: Jacob Klegar <[email protected]>
Signed-off-by: Jacob Klegar <[email protected]>
Signed-off-by: Jacob Klegar <[email protected]>
Signed-off-by: Jacob Klegar <[email protected]>
jklegar
force-pushed
the
jacob_fix_materialization_tracking
branch
from
ac23456 to
61b6924
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, jklegar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: