-
Notifications
You must be signed in to change notification settings - Fork 982
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
fix: Added Apply/Get/List/Delete methods to project_metadata to BaseRegistry #4441
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Bhargav Dodla <[email protected]>
Signed-off-by: Bhargav Dodla <[email protected]>
Signed-off-by: Bhargav Dodla <[email protected]>
@@ -545,22 +545,72 @@ def list_validation_references( | |||
""" | |||
raise NotImplementedError | |||
|
|||
@abstractmethod | |||
def apply_project_metadata( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 Another point... if we treat |
Signed-off-by: Bhargav Dodla <[email protected]>
… and using default string Signed-off-by: Bhargav Dodla <[email protected]>
Signed-off-by: Bhargav Dodla <[email protected]>
@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. |
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. |
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 @franciscojavierarceo @HaoXuAI Any thoughts/objections about introducing |
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? |
@EXPEbdodla this is no longer relevant, right? |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Misc