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

FeastObject type is missing SavedDataset #4312

Closed
dmartinol opened this issue Jun 24, 2024 · 5 comments · Fixed by #4315
Closed

FeastObject type is missing SavedDataset #4312

dmartinol opened this issue Jun 24, 2024 · 5 comments · Fixed by #4315

Comments

@dmartinol
Copy link
Contributor

Expected Behavior

FeastObject should include all types of instances applied to the feature store.

Current Behavior

It misses SavedDataset type.

Specifications

  • Version: 0.39.0
@dmartinol
Copy link
Contributor Author

@tokoko

@tokoko
Copy link
Collaborator

tokoko commented Jun 24, 2024

Agreed, I'd add that although class BatchFeatureView exists in the codebase, it's basically unused at this point and not part of registry CRUD operations. Even if it was, once #4235 gets resolved, do you think it's still a good idea not to treat FeatureViews as a single object type?

@dmartinol
Copy link
Contributor Author

Can I take this one and send a PR?

@tokoko
Copy link
Collaborator

tokoko commented Jun 25, 2024

sure, go ahead

@dmartinol
Copy link
Contributor Author

Even if it was, once #4235 gets resolved, do you think it's still a good idea not to treat FeatureViews as a single object type?

I'm not against having a single type but I cannot estimate the impacts on the registry (endpoints and diff logic).
In the permission model we're using a filter to allow selecting only specific types and exclude sub-classes, which again should be reviewed.

Pls note that OnDemandFeatureView is a BaseFeatureView only, this would either need its entry in FeastObject or to review its hierarchy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants