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

Don't allow FeatureStore.apply with commit=False #2047

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,22 +391,20 @@ def apply(
]
] = [],
partial: bool = True,
commit: bool = True,
):
"""Register objects to metadata store and update related infrastructure.

The apply method registers one or more definitions (e.g., Entity, FeatureView) and registers or updates these
objects in the Feast registry. Once the registry has been updated, the apply method will update related
infrastructure (e.g., create tables in an online store) in order to reflect these new definitions. All
operations are idempotent, meaning they can safely be rerun.
objects in the Feast registry. Once the apply method has updated the infrastructure (e.g., create tables in
an online store), it will commit the updated registry. All operations are idempotent, meaning they can safely
be rerun.

Args:
objects: A single object, or a list of objects that should be registered with the Feature Store.
objects_to_delete: A list of objects to be deleted from the registry and removed from the
provider's infrastructure. This deletion will only be performed if partial is set to False.
partial: If True, apply will only handle the specified objects; if False, apply will also delete
all the objects in objects_to_delete, and tear down any associated cloud resources.
commit: whether to commit changes to the registry

Raises:
ValueError: The 'objects' parameter could not be parsed properly.
Expand Down Expand Up @@ -501,9 +499,13 @@ def apply(
for ent in entities_to_update:
self._registry.apply_entity(ent, project=self.project, commit=False)
for feature_service in services_to_update:
self._registry.apply_feature_service(feature_service, project=self.project)
self._registry.apply_feature_service(
feature_service, project=self.project, commit=False
)
for table in tables_to_update:
self._registry.apply_feature_table(table, project=self.project)
self._registry.apply_feature_table(
table, project=self.project, commit=False
)

if not partial:
# Delete all registry objects that should not exist.
Expand Down Expand Up @@ -560,8 +562,7 @@ def apply(
partial=partial,
)

if commit:
self._registry.commit()
self._registry.commit()

@log_exceptions_and_usage
def teardown(self):
Expand Down
7 changes: 1 addition & 6 deletions sdk/python/feast/repo_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ def apply_total(repo_config: RepoConfig, repo_path: Path, skip_source_validation
all_to_delete.extend(odfvs_to_delete)
all_to_delete.extend(tables_to_delete)

store.apply(
all_to_apply, objects_to_delete=all_to_delete, partial=False, commit=False
)
store.apply(all_to_apply, objects_to_delete=all_to_delete, partial=False)

for entity in entities_to_delete:
click.echo(
Expand Down Expand Up @@ -258,9 +256,6 @@ def apply_total(repo_config: RepoConfig, repo_path: Path, skip_source_validation
)
# TODO: consider echoing also entities being deployed/removed

# Commit the update to the registry only after successful infra update
registry.commit()


def _tag_registry_entities_for_keep_delete(
project: str, registry: Registry, repo: ParsedRepo
Expand Down