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

fix: Run store.plan() only when need it. #3708

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

shuchu
Copy link
Collaborator

@shuchu shuchu commented Aug 3, 2023

What this PR does / why we need it:

This line of code:
registry_diff, infra_diff, new_infra = store.plan(repo)

is always executed while we run "feast apply". It will call the validate() method by default which makes the option "-skip-source-validation" invalid.

Based on the original code logic, I change it to run it only when it is required. The requirement comes from:
if store._should_use_plan():

In the implementation of _should_use_plan():

def _should_use_plan(self):   
"""Returns True if plan and _apply_diffs should be used, False otherwise."""   
"""# Currently only the local provider with sqlite online store supports plan and _apply_diffs. """  
return self.config.provider == "local" and ( self.config.online_store and self.config.online_store.type == "sqlite" )`  

the "store._should_use_plan()" return True while the provider is "local" and the online store is "sqlite".

With this PR, we can alleviate the issue 3682. If a user wants to do local development, he/she may want to change the online store to a different one other than "SQLite", the action of double call "store.validate()" can be avoided.

Which issue(s) this PR fixes:
Fixes # 3682

@shuchu shuchu changed the title fix: issue-3682, run store.plan() only when need it. fix: run store.plan() only when need it. Aug 3, 2023
@shuchu shuchu changed the title fix: run store.plan() only when need it. fix: Run store.plan() only when need it. Aug 3, 2023
Copy link
Collaborator

@felixwang9817 felixwang9817 left a comment

Choose a reason for hiding this comment

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

/lgtm

@felixwang9817 felixwang9817 merged commit 7bc7c47 into feast-dev:master Aug 14, 2023
felixwang9817 pushed a commit that referenced this pull request Aug 14, 2023
## [0.33.1](v0.33.0...v0.33.1) (2023-08-14)

### Bug Fixes

* Run store.plan() only when need it. ([#3708](#3708)) ([7bc7c47](7bc7c47))
adchia pushed a commit that referenced this pull request Sep 7, 2023
# [0.34.0](v0.33.0...v0.34.0) (2023-09-07)

### Bug Fixes

* Add NUMERIC to bq_to_feast type map ([#3719](#3719)) ([6474b4b](6474b4b))
* Fix python unit tests ([#3734](#3734)) ([e81684d](e81684d))
* Handle unknown postgres source types gracefully ([#3634](#3634)) ([d7041f4](d7041f4))
* Pin protobuf version to avoid seg fault on some machines ([028cc20](028cc20))
* Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([#3730](#3730)) ([f2c5988](f2c5988))
* Run store.plan() only when need it. ([#3708](#3708)) ([7bc7c47](7bc7c47))
* Saved datasets no longer break CLI registry-dump command ([#3717](#3717)) ([f28ccc2](f28ccc2))
* Update py3.8 ci requirements for cython 3.0 release ([#3735](#3735)) ([1695c13](1695c13))

### Features

* Enhance customization of Trino connections when using Trino-based Offline Stores ([#3699](#3699)) ([ed7535e](ed7535e))
* Implement gRPC server to ingest streaming features ([#3687](#3687)) ([a3fcd1f](a3fcd1f))
james-crabtree-sp pushed a commit to sailpoint/feast that referenced this pull request Sep 14, 2023
# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07)

### Bug Fixes

* Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b))
* Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d))
* Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4))
* Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20))
* Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988))
* Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47))
* Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2))
* Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13))

### Features

* Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e))
* Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f))
james-crabtree-sp pushed a commit to sailpoint/feast that referenced this pull request Sep 14, 2023
# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07)

### Bug Fixes

* Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b))
* Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d))
* Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4))
* Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20))
* Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988))
* Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47))
* Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2))
* Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13))

### Features

* Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e))
* Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f))
zseta pushed a commit to zseta/feast that referenced this pull request Feb 7, 2024
fix: issue-3682, run store.plan() only when need it.
Signed-off-by: Attila Toth <hello@attilatoth.dev>
zseta pushed a commit to zseta/feast that referenced this pull request Feb 7, 2024
# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07)

### Bug Fixes

* Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b))
* Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d))
* Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4))
* Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20))
* Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988))
* Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47))
* Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2))
* Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13))

### Features

* Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e))
* Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f))

Signed-off-by: Attila Toth <hello@attilatoth.dev>
EXPEbdodla pushed a commit to ExpediaGroup/feast that referenced this pull request Jun 10, 2024
fix: issue-3682, run store.plan() only when need it.
EXPEbdodla pushed a commit to ExpediaGroup/feast that referenced this pull request Jun 11, 2024
fix: issue-3682, run store.plan() only when need it.
EXPEbdodla added a commit to ExpediaGroup/feast that referenced this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants