Skip to content

fix: Delete saved datasets passed to apply via objects_to_delete - #7045

Open
Vaatsal-source wants to merge 1 commit into
feast-dev:masterfrom
Vaatsal-source:fix/delete-saved-dataset
Open

Vaatsal-source wants to merge 1 commit into
feast-dev:masterfrom
Vaatsal-source:fix/delete-saved-dataset

Conversation

@Vaatsal-source

Copy link
Copy Markdown

What this PR does / why we need it:

FeatureStore.apply(..., objects_to_delete=[...], partial=False) sorts the objects to delete by type, but it had no case for SavedDataset, so saved datasets were silently ignored. Because feast delete <name> deletes through apply(), the CLI printed Deleted SavedDataset <name> while the dataset stayed in the registry.

This PR collects SavedDataset objects from objects_to_delete and removes them with the existing registry.delete_saved_dataset(...), following the same pattern used for validation references. delete_saved_dataset already exists on all registry backends, so no registry changes are needed.

Which issue(s) this PR fixes:

Fixes #7044

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Added test_apply_delete_saved_dataset in test_local_feature_store.py. It registers a saved dataset, deletes it via apply(objects=[], objects_to_delete=[saved_dataset], partial=False), and asserts it is gone. The test fails without the fix.

Manually verified with a local repo that feast delete <saved_dataset> now removes the saved dataset from the registry.

Misc

No docs change needed: the feast delete reference already lists saved datasets as supported.

@Vaatsal-source
Vaatsal-source requested a review from a team as a code owner October 19, 2026 08:12

@shuchu shuchu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@codecov-commenter

codecov-commenter commented Nov 10, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.46%. Comparing base (ce02644) to head (4a39452).
⚠️ Report is 1 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7045      +/-   ##
==========================================
+ Coverage   52.45%   52.46%   +0.01%     
==========================================
  Files         459      459              
  Lines       58172    58175       +3     
  Branches     8577     8578       +1     
==========================================
+ Hits        30515    30523       +8     
+ Misses      25573    25565       -8     
- Partials     2084     2087       +3     
Flag Coverage Δ
go-feature-server 30.96% <ø> (ø)
python-unit 53.97% <100.00%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/feature_store.py 47.68% <100.00%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce02644...4a39452. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

FeatureStore.apply() ignored SavedDataset objects in objects_to_delete, so 'feast delete <saved_dataset>' reported success without removing the dataset from the registry.

Signed-off-by: Vaatsalya Srivastava <vaatsalyasrivastava_ee_25a19036@dtu.ac.in>
@ntkathole
ntkathole force-pushed the fix/delete-saved-dataset branch from 1ceed3e to 4a39452 Compare November 13, 2026 14:22

This branch has not been deployed

No deployments
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.

feast delete reports success for a SavedDataset but does not remove it from the registry

4 participants