fix: Added Apply/Get/List/Delete methods to project_metadata to BaseRegistry by EXPEbdodla · Pull Request #4441 · feast-dev/feast
What this PR does / why we need it:
- Adds Apply/Get/List/Delete methods to project_metadata to BaseRegistry
- Made project as Optional in list_project_metadata method so we can use it retrieve all projects when project is None. Can be deleted in future if necessary.
- Renamed _maybe_init_project_metadata to apply_project_metadata. Registry Server will have use cases to create projects in future.
- I'll implement the methods to remote registry in future PRs
Which issue(s) this PR fixes:
Misc
Bhargav Dodla added 2 commits
Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
Add project endpoints
fix: Added Apply/Get/List/Delete methods to project_metadata to BaseRegistry
Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
| raise NotImplementedError | ||
|
|
||
| @abstractmethod | ||
| def apply_project_metadata( |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make more sense for this to also accept ProjectMetadata object as an input similar to other registry resources? I know the object doesn't hold much information right now, but if we plan for it to be "updatable", an update should happen through this method, right?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not envisioning this as a updatable. At this point its create only. Fields we have for this are UUID and LastUpdateTimestamp. UUID should be constant. LastUpdateTimestamp should be update when any changes happen to any objects associated to the project.
This probably means we will have to treat ProjectMetadata as another one of FeastObject types and should be added in here. Similarly for rbac to work, we need it here as well, but that can be deferred until handling in remote registry is implemented. @dmartinol
Another point... if we treat apply_project_metadata as basically project creation and delete_project_metadata as deletion, along with all the contents (implementations seem to indicate this), wouldn't better name for this new feast object be Project rather than ProjectMetadata?
Bhargav Dodla added 3 commits
Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
… and using default string Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
Signed-off-by: Bhargav Dodla <bdodla@expediagroup.com>
Another point... if we treat apply_project_metadata as basically project creation and delete_project_metadata as deletion, along with all the contents (implementations seem to indicate this), wouldn't better name for this new feast object be Project rather than ProjectMetadata?
@tokoko You are right. We can introduce the Project object with spec and metadata. We can use the Project and fade out ProjectMetadata. Should I proceed with changing the PR accordingly?
And I always saw Project as a parent entity for all the FEAST Objects.
Having a Separate Project object will also help us to load or preserve the feature_store.yaml associated to the project as we progress further.
This probably means we will have to treat ProjectMetadata as another one of FeastObject types and should be added in here.
We won't treat this as an object that users define in Feature Definitions. We can populate project name from feature_store.yaml. At this point, I'm not aware of any use cases for this.
If we let users define Project in definitions, we need to ensure name in feature_store.yaml match with Project object in definitions and ensure we can only have one Project is defined per Repo. This will enable users to specify/ update tags, owner, description, any other metadata we would like to add.
Defining Project object may be a case for RBAC. I have no idea about this at this point. Looking forward for the suggestions on this.
Defining Project object may be a case for RBAC. I have no idea about this at this point. Looking forward for the suggestions on this.
yup, that's what I think as well. A lot of rbac revolves around tagging, so it makes sense to make it possible to tag projects and consequently we will have to let users update those tags. Maybe we can make it so that users are able to define Project objects in the repo, but revert to auto-generating one for them if they don't do it.
@franciscojavierarceo @HaoXuAI Any thoughts/objections about introducing Project feast object and phasing out ProjectMetadata in favor of it? I think it makes a lot of sense especially considering rbac.
This probably means we will have to treat ProjectMetadata as another one of FeastObject types and should be added in here. Similarly for rbac to work, we need it here as well, but that can be deferred until handling in remote registry is implemented. @dmartinol
Agreee, while developing the registry_server we'll also need to assert the required permissions.
No objection. Can we have multiple projects per repo where different project use different online/offline store setup?
Is this PR still relevant? Thanks for doing this @EXPEbdodla and sorry for the complications here.
Not relevant. I'll close it.