-
Notifications
You must be signed in to change notification settings - Fork 1.2k
docs: Clarify that feast apply does not delete objects and document feast delete CLI #5854
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
Conversation
…le modifications Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
sdk/python/feast/ui_server.py
Outdated
| # 3. This ensures deleted feature views (via Python SDK) are immediately detected | ||
| # 4. This is internal Feast code, so using internal APIs is acceptable | ||
| current_registry_proto = store.registry._get_registry_proto( | ||
| project=None, allow_cache=True |
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.
why are we pulling when project=None, feels like that won't return anything useful.
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.
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.
Changed to use store.project instead of None for better semantic clarity. While the project parameter isn't used in the current Registry implementation logic (it returns the full registry proto regardless), passing the actual project makes the intent clearer. Commit: 8f66298
You say this but how do you know? I don't see a test for this that confirms it. Also, please run |
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
franciscojavierarceo
left a comment
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.
These changes aren't necessary.
After looking more at the user's example, it was a poor documentation.
The user expects that commenting out a feature and running feast apply or FeatureStore.apply([_subset_of_feast_objects_]) would delete previously registered objects. That's not the expected behavior.
Users have to explicitly delete objects in Feast and we have a set of class methods to support that. Instead, @copilot, can you update the documentation for the registry (or where appropriate) that deleting objects in the feast registry requires running the explicit delete commands. Please review the different class methods (e.g., FeatureStore.delete_entity, FeatureStore.delete_data_source, etc.).
…n requirements Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Reverted all code changes and added comprehensive documentation clarifying that deletion requires explicit methods. Updated:
All changes in commit cf1b528.
After review, the original issue was a documentation problem, not a bug. Users expected |
|
@copilot can you also update the CLI documentation? |
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Added comprehensive CLI documentation for the
Commit 5195dc9. |
franciscojavierarceo
left a comment
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.
nice!
What this PR does / why we need it:
This PR addresses a common misunderstanding about Feast's
applybehavior. Users expect that removing feature definitions from their code and runningfeast applyorFeatureStore.apply()will delete those objects from the registry, but that's not how Feast works. Objects must be explicitly deleted using dedicated delete methods or CLI commands.This PR adds comprehensive documentation to clarify this behavior and guide users on the correct way to delete objects from the Feast registry, including full documentation for the
feast deleteCLI command.Changes Made
Documentation Updates
feast applydoesn't delete objectsfeast deletecommand examplesdelete_feature_view()anddelete_feature_service()feast deletecommand to the commands overviewfeast applysection clarifying that it only registers/updates objects found in code, with reference to delete commandapply()method docstring to explicitly state it doesn't delete objects removed from the provided listCode Quality
make format-python- 670 files left unchangedmake lint-python- All checks passedMisc
The original issue report was based on a misunderstanding of Feast's expected behavior. The documentation now clearly explains that deletion requires explicit method calls (via Python SDK) or the
feast deleteCLI command rather than implicit removal throughapply.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.