Add a feast plan command, and have CLI output differentiates between created, deleted and unchanged objects #2147
Conversation
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
achals
changed the title
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
|
Does this also distinguish Updated from Created? |
The plan is to do that in a separate PR - but yes, we should be able to infer and print out updated fields for objects. This pr doesn't really grok in place updates to objects yet. |
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
Signed-off-by: Achal Shah <[email protected]>
sdk/python/feast/feature_store.py
Outdated
| ] | ||
| ] = None, | ||
| partial: bool = True, | ||
| ): |
There was a problem hiding this comment.
nit: return type should be diff
sdk/python/feast/feature_store.py
Outdated
| @@ -358,7 +360,7 @@ def _get_features(self, features: Union[List[str], FeatureService],) -> List[str | |||
| return _feature_refs | |||
|
|
|||
| @log_exceptions_and_usage | |||
| def apply( | |||
| def plan( | |||
There was a problem hiding this comment.
it's extremely unintuitive to me that plan already has a concept of objects to keep and delete - after all, the point of planning should be to determine which things to keep, add, and delete
what I had in mind with plan was to reconstruct a full RegistryProto from a ParsedRepo, and then compare that with the existing registry - thoughts on this approach vs. yours?
There was a problem hiding this comment.
Okay, I think that make sense. Lemme update.
Signed-off-by: Achal Shah <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2147 +/- ##
==========================================
+ Coverage 84.60% 84.70% +0.09%
==========================================
Files 101 101
Lines 7966 8035 +69
==========================================
+ Hits 6740 6806 +66
- Misses 1226 1229 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, felixwang9817 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 |
| @@ -128,32 +129,85 @@ def __init__( | |||
| else 0 | |||
| ) | |||
|
|
|||
| def clone(self) -> "Registry": | |||
| new_registry = Registry(None, None) | |||
| new_registry.cached_registry_proto_ttl = timedelta(seconds=0) | |||
There was a problem hiding this comment.
Why set the new_registry.cached_registry_proto_ttl to 0 seconds?
There was a problem hiding this comment.
@judahrand I'm actually not sure why, but we don't actually use this clone method anymore (I'll delete it in a future PR)
Signed-off-by: Achal Shah [email protected]
What this PR does / why we need it:
Follow up to #2142 , this method uses the info from the refactored tag methods to print out new output.
Does this PR introduce a user-facing change?: