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.

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

Set created_timestamp and last_updated_timestamp fields #2266

Merged
merged 6 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 3 additions & 0 deletions protos/feast/core/OnDemandFeatureView.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ message OnDemandFeatureViewSpec {
message OnDemandFeatureViewMeta {
// Time where this Feature View is created
google.protobuf.Timestamp created_timestamp = 1;

// Time where this Feature View is last updated
google.protobuf.Timestamp last_updated_timestamp = 2;
}

message OnDemandInput {
Expand Down
4 changes: 4 additions & 0 deletions sdk/python/feast/base_feature_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@
class BaseFeatureView(ABC):
"""A FeatureView defines a logical grouping of features to be served."""

created_timestamp: Optional[datetime] = None
last_updated_timestamp: Optional[datetime] = None

@abstractmethod
def __init__(self, name: str, features: List[Feature]):
self._name = name
self._features = features
self._projection = FeatureViewProjection.from_definition(self)
self.created_timestamp: Optional[datetime] = None
self.last_updated_timestamp: Optional[datetime] = None

@property
def name(self) -> str:
Expand Down
1 change: 1 addition & 0 deletions sdk/python/feast/feature_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class FeatureView(BaseFeatureView):
input: DataSource
batch_source: DataSource
stream_source: Optional[DataSource] = None
created_timestamp: Optional[datetime] = None
last_updated_timestamp: Optional[datetime] = None
materialization_intervals: List[Tuple[datetime, datetime]]

Expand Down
13 changes: 12 additions & 1 deletion sdk/python/feast/on_demand_feature_view.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import copy
import functools
from datetime import datetime
from types import MethodType
from typing import Dict, List, Type, Union
from typing import Dict, List, Optional, Type, Union

import dill
import pandas as pd
Expand Down Expand Up @@ -47,6 +48,8 @@ class OnDemandFeatureView(BaseFeatureView):
input_feature_view_projections: Dict[str, FeatureViewProjection]
input_request_data_sources: Dict[str, RequestDataSource]
udf: MethodType
created_timestamp: Optional[datetime] = None
Copy link
Member

Choose a reason for hiding this comment

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

The args above arent updated btw

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean? The other FeatureView classes don't take it in __init__. I was just making sure they all matched in terms of behaviour.

Arguably, this setting of created_timestamp to None should maybe be inherited behaviour.

Copy link
Member

@woop woop Feb 2, 2022

Choose a reason for hiding this comment

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

I mean the docstring. I also think this logic should ideally be inherited.

Copy link
Member Author

Choose a reason for hiding this comment

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

Still not sure what you mean 🤔 The Docstring shouldn't mention this should it as it isn't an arg?

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out this logic was already performed by the parent class. The logic must've been left in accidentally.

last_updated_timestamp: Optional[datetime] = None

@log_exceptions
def __init__(
Expand All @@ -71,6 +74,8 @@ def __init__(
self.input_feature_view_projections[input_ref] = odfv_input.projection

self.udf = udf
self.created_timestamp: Optional[datetime] = None
self.last_updated_timestamp: Optional[datetime] = None

@property
def proto_class(self) -> Type[OnDemandFeatureViewProto]:
Expand Down Expand Up @@ -119,6 +124,8 @@ def to_proto(self) -> OnDemandFeatureViewProto:
meta = OnDemandFeatureViewMeta()
if self.created_timestamp:
meta.created_timestamp.FromDatetime(self.created_timestamp)
if self.last_updated_timestamp:
meta.last_updated_timestamp.FromDatetime(self.last_updated_timestamp)
inputs = {}
for input_ref, fv_projection in self.input_feature_view_projections.items():
inputs[input_ref] = OnDemandInput(
Expand Down Expand Up @@ -194,6 +201,10 @@ def from_proto(cls, on_demand_feature_view_proto: OnDemandFeatureViewProto):
on_demand_feature_view_obj.created_timestamp = (
on_demand_feature_view_proto.meta.created_timestamp.ToDatetime()
)
if on_demand_feature_view_proto.meta.HasField("last_updated_timestamp"):
on_demand_feature_view_obj.last_updated_timestamp = (
on_demand_feature_view_proto.meta.last_updated_timestamp.ToDatetime()
)

return on_demand_feature_view_obj

Expand Down
23 changes: 22 additions & 1 deletion sdk/python/feast/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,12 @@ def apply_entity(self, entity: Entity, project: str, commit: bool = True):
commit: Whether the change should be persisted immediately
"""
entity.is_valid()

now = datetime.utcnow()
if not entity.created_timestamp:
entity._created_timestamp = now
entity._last_updated_timestamp = now

entity_proto = entity.to_proto()
entity_proto.spec.project = project
self._prepare_registry_for_changes()
Expand Down Expand Up @@ -278,6 +284,11 @@ def apply_feature_service(
feature_service: A feature service that will be registered
project: Feast project that this entity belongs to
"""
now = datetime.utcnow()
if not feature_service.created_timestamp:
feature_service.created_timestamp = now
feature_service.last_updated_timestamp = now

feature_service_proto = feature_service.to_proto()
feature_service_proto.spec.project = project

Expand Down Expand Up @@ -373,8 +384,12 @@ def apply_feature_view(
commit: Whether the change should be persisted immediately
"""
feature_view.ensure_valid()

now = datetime.utcnow()
if not feature_view.created_timestamp:
feature_view.created_timestamp = datetime.utcnow()
feature_view.created_timestamp = now
feature_view.last_updated_timestamp = now

feature_view_proto = feature_view.to_proto()
feature_view_proto.spec.project = project
self._prepare_registry_for_changes()
Expand Down Expand Up @@ -498,6 +513,7 @@ def apply_materialization(
existing_feature_view.materialization_intervals.append(
(start_date, end_date)
)
existing_feature_view.last_updated_timestamp = datetime.utcnow()
feature_view_proto = existing_feature_view.to_proto()
feature_view_proto.spec.project = project
del self.cached_registry_proto.feature_views[idx]
Expand Down Expand Up @@ -686,6 +702,11 @@ def apply_saved_dataset(
project: Feast project that this dataset belongs to
commit: Whether the change should be persisted immediately
"""
now = datetime.utcnow()
if not saved_dataset.created_timestamp:
saved_dataset.created_timestamp = now
saved_dataset.last_updated_timestamp = now

saved_dataset_proto = saved_dataset.to_proto()
saved_dataset_proto.spec.project = project
self._prepare_registry_for_changes()
Expand Down