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

feat: Updating OnDemandFeatureView to add Entities and batch_source #4530

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

franciscojavierarceo
Copy link
Member

@franciscojavierarceo franciscojavierarceo commented Sep 17, 2024

What this PR does / why we need it:

This PR

  1. Updates FeatureViewProjection to include the underlying data sources for OnDemandFeatureView. This allows for much richer lineage when constructing metadata about an OnDemandFeatureViews and its data sources.
  2. Updates OnDemandFeatureView to optionally support entities, which will be required to write an OnDemandFeatureView to the online store
  3. Updates BaseFeatureView to include sources to optionally include bath_source
  4. Add write_to_online_store boolean to OnDemandFeatureView (note not used)

Which issue(s) this PR fixes:

Misc

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
…otobuf

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
name_alias=None,
features=base_feature_view.features,
desired_features=[],
timestamp_field=base_feature_view.batch_source.created_timestamp_column # type:ignore[attr-defined]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is admittedly a hack and I don't like it but this should be refactored in 1.0.0

My overall learning from this is that FeatureViews should be really derived from the same object with the same class parameters and make them optionally instantiated.

This should be described more thoroughly for 1.0.0

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
@franciscojavierarceo franciscojavierarceo changed the title feat: Updating protos for Projections to include more info feat: Updating OnDemandFeatureView to add Entities and batch_source Sep 18, 2024
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
…ing to put a workaround

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant