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

feat: Move get_online_features to OnlineStore interface #4319

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

tokoko
Copy link
Collaborator

@tokoko tokoko commented Jun 27, 2024

Which issue(s) this PR fixes:

Fixes #4270

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
@tokoko tokoko requested review from DvirDukhan and a team as code owners June 27, 2024 00:38
@tokoko tokoko requested review from pyalex and removed request for a team June 27, 2024 00:38
Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
@tokoko tokoko requested a review from HaoXuAI June 27, 2024 14:33
@@ -105,6 +109,180 @@ async def online_read_async(
f"Online store {self.__class__.__name__} does not support online read async"
)

def get_online_features(
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this a good idea? I thought OnlineStore is interface only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now it is, but it's clearly not an ideal one. If we leave everything else aside, just the fact that it exposes read on feature view level rather than a request level means that we need to change it somehow because it's impossible to read data from the online store in a single round-trip. The solution here leaves the abstract methods intact in order to keep things backwards-compatible while also exposing another higher-level method with default implementation that enables other online stores to improve on performance. I don't think keeping it as a strictly abstract interface matters all that much.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, just curious what was the intention before when the interface was defined? anyway, I think we can go with the new pattern.

@HaoXuAI HaoXuAI merged commit 7072fd0 into feast-dev:master Jul 1, 2024
15 checks passed
tsisodia10 pushed a commit to tsisodia10/feast that referenced this pull request Jul 23, 2024
* move get_online_features to OnlineStore interface

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* fix pydantic warnings

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* run ruff format

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

---------

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
tsisodia10 pushed a commit to tsisodia10/feast that referenced this pull request Jul 23, 2024
* move get_online_features to OnlineStore interface

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* fix pydantic warnings

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* run ruff format

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

---------

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
nick-amaya-sp pushed a commit to sailpoint/feast that referenced this pull request Jul 23, 2024
* move get_online_features to OnlineStore interface

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* fix pydantic warnings

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

* run ruff format

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

---------

Signed-off-by: tokoko <togurg14@freeuni.edu.ge>
franciscojavierarceo pushed a commit that referenced this pull request Jul 31, 2024
# [0.40.0](v0.39.0...v0.40.0) (2024-07-31)

### Bug Fixes

* Added missing type ([#4315](#4315)) ([86af60a](86af60a))
* Avoid XSS attack from Jinjin2's Environment(). ([#4355](#4355)) ([40270e7](40270e7))
* CGO Memory leak issue in GO Feature server ([#4291](#4291)) ([43e198f](43e198f))
* Deprecated the datetime.utcfromtimestamp(). ([#4306](#4306)) ([21deec8](21deec8))
* Fix SQLite import issue ([#4294](#4294)) ([398ea3b](398ea3b))
* Increment operator to v0.39.0 ([#4368](#4368)) ([3ddb4fb](3ddb4fb))
* Minor typo in the unit test. ([#4296](#4296)) ([6c75e84](6c75e84))
* OnDemandFeatureView type inference for array types ([#4310](#4310)) ([c45ff72](c45ff72))
* Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([#4331](#4331)) ([0d89d15](0d89d15))
* Remove typo. ([#4351](#4351)) ([92d17de](92d17de))
* Retire the datetime.utcnow(). ([#4352](#4352)) ([a8bc696](a8bc696))
* Update dask version to support pandas 1.x ([#4326](#4326)) ([a639d61](a639d61))
* Update Feast object metadata in the registry ([#4257](#4257)) ([8028ae0](8028ae0))
* Using one single function call for utcnow(). ([#4307](#4307)) ([98ff63c](98ff63c))

### Features

* Add async feature retrieval for Postgres Online Store ([#4327](#4327)) ([cea52e9](cea52e9))
* Add Async refresh to Sql Registry ([#4251](#4251)) ([f569786](f569786))
* Add SingleStore as an OnlineStore ([#4285](#4285)) ([2c38946](2c38946))
* Add Tornike to maintainers.md ([#4339](#4339)) ([8e8c1f2](8e8c1f2))
* Bump psycopg2 to psycopg3 for all Postgres components ([#4303](#4303)) ([9451d9c](9451d9c))
* Entity key deserialization ([#4284](#4284)) ([83fad15](83fad15))
* Ignore paths feast apply ([#4276](#4276)) ([b4d54af](b4d54af))
* Move get_online_features to OnlineStore interface ([#4319](#4319)) ([7072fd0](7072fd0))
* Port mssql contrib offline store to ibis ([#4360](#4360)) ([7914cbd](7914cbd))

### Reverts

* Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([#4357](#4357)) ([cdeab48](cdeab48)), closes [#4355](#4355)
redhatHameed pushed a commit to RHEcosystemAppEng/feast that referenced this pull request Aug 5, 2024
# [0.40.0](feast-dev/feast@v0.39.0...v0.40.0) (2024-07-31)

### Bug Fixes

* Added missing type ([feast-dev#4315](feast-dev#4315)) ([86af60a](feast-dev@86af60a))
* Avoid XSS attack from Jinjin2's Environment(). ([feast-dev#4355](feast-dev#4355)) ([40270e7](feast-dev@40270e7))
* CGO Memory leak issue in GO Feature server ([feast-dev#4291](feast-dev#4291)) ([43e198f](feast-dev@43e198f))
* Deprecated the datetime.utcfromtimestamp(). ([feast-dev#4306](feast-dev#4306)) ([21deec8](feast-dev@21deec8))
* Fix SQLite import issue ([feast-dev#4294](feast-dev#4294)) ([398ea3b](feast-dev@398ea3b))
* Increment operator to v0.39.0 ([feast-dev#4368](feast-dev#4368)) ([3ddb4fb](feast-dev@3ddb4fb))
* Minor typo in the unit test. ([feast-dev#4296](feast-dev#4296)) ([6c75e84](feast-dev@6c75e84))
* OnDemandFeatureView type inference for array types ([feast-dev#4310](feast-dev#4310)) ([c45ff72](feast-dev@c45ff72))
* Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([feast-dev#4331](feast-dev#4331)) ([0d89d15](feast-dev@0d89d15))
* Remove typo. ([feast-dev#4351](feast-dev#4351)) ([92d17de](feast-dev@92d17de))
* Retire the datetime.utcnow(). ([feast-dev#4352](feast-dev#4352)) ([a8bc696](feast-dev@a8bc696))
* Update dask version to support pandas 1.x ([feast-dev#4326](feast-dev#4326)) ([a639d61](feast-dev@a639d61))
* Update Feast object metadata in the registry ([feast-dev#4257](feast-dev#4257)) ([8028ae0](feast-dev@8028ae0))
* Using one single function call for utcnow(). ([feast-dev#4307](feast-dev#4307)) ([98ff63c](feast-dev@98ff63c))

### Features

* Add async feature retrieval for Postgres Online Store ([feast-dev#4327](feast-dev#4327)) ([cea52e9](feast-dev@cea52e9))
* Add Async refresh to Sql Registry ([feast-dev#4251](feast-dev#4251)) ([f569786](feast-dev@f569786))
* Add SingleStore as an OnlineStore ([feast-dev#4285](feast-dev#4285)) ([2c38946](feast-dev@2c38946))
* Add Tornike to maintainers.md ([feast-dev#4339](feast-dev#4339)) ([8e8c1f2](feast-dev@8e8c1f2))
* Bump psycopg2 to psycopg3 for all Postgres components ([feast-dev#4303](feast-dev#4303)) ([9451d9c](feast-dev@9451d9c))
* Entity key deserialization ([feast-dev#4284](feast-dev#4284)) ([83fad15](feast-dev@83fad15))
* Ignore paths feast apply ([feast-dev#4276](feast-dev#4276)) ([b4d54af](feast-dev@b4d54af))
* Move get_online_features to OnlineStore interface ([feast-dev#4319](feast-dev#4319)) ([7072fd0](feast-dev@7072fd0))
* Port mssql contrib offline store to ibis ([feast-dev#4360](feast-dev#4360)) ([7914cbd](feast-dev@7914cbd))

### Reverts

* Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([feast-dev#4357](feast-dev#4357)) ([cdeab48](feast-dev@cdeab48)), closes [feast-dev#4355](feast-dev#4355)
shuchu pushed a commit to shuchu/feast that referenced this pull request Aug 14, 2024
# [0.40.0](feast-dev/feast@v0.39.0...v0.40.0) (2024-07-31)

### Bug Fixes

* Added missing type ([feast-dev#4315](feast-dev#4315)) ([86af60a](feast-dev@86af60a))
* Avoid XSS attack from Jinjin2's Environment(). ([feast-dev#4355](feast-dev#4355)) ([40270e7](feast-dev@40270e7))
* CGO Memory leak issue in GO Feature server ([feast-dev#4291](feast-dev#4291)) ([43e198f](feast-dev@43e198f))
* Deprecated the datetime.utcfromtimestamp(). ([feast-dev#4306](feast-dev#4306)) ([21deec8](feast-dev@21deec8))
* Fix SQLite import issue ([feast-dev#4294](feast-dev#4294)) ([398ea3b](feast-dev@398ea3b))
* Increment operator to v0.39.0 ([feast-dev#4368](feast-dev#4368)) ([3ddb4fb](feast-dev@3ddb4fb))
* Minor typo in the unit test. ([feast-dev#4296](feast-dev#4296)) ([6c75e84](feast-dev@6c75e84))
* OnDemandFeatureView type inference for array types ([feast-dev#4310](feast-dev#4310)) ([c45ff72](feast-dev@c45ff72))
* Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([feast-dev#4331](feast-dev#4331)) ([0d89d15](feast-dev@0d89d15))
* Remove typo. ([feast-dev#4351](feast-dev#4351)) ([92d17de](feast-dev@92d17de))
* Retire the datetime.utcnow(). ([feast-dev#4352](feast-dev#4352)) ([a8bc696](feast-dev@a8bc696))
* Update dask version to support pandas 1.x ([feast-dev#4326](feast-dev#4326)) ([a639d61](feast-dev@a639d61))
* Update Feast object metadata in the registry ([feast-dev#4257](feast-dev#4257)) ([8028ae0](feast-dev@8028ae0))
* Using one single function call for utcnow(). ([feast-dev#4307](feast-dev#4307)) ([98ff63c](feast-dev@98ff63c))

### Features

* Add async feature retrieval for Postgres Online Store ([feast-dev#4327](feast-dev#4327)) ([cea52e9](feast-dev@cea52e9))
* Add Async refresh to Sql Registry ([feast-dev#4251](feast-dev#4251)) ([f569786](feast-dev@f569786))
* Add SingleStore as an OnlineStore ([feast-dev#4285](feast-dev#4285)) ([2c38946](feast-dev@2c38946))
* Add Tornike to maintainers.md ([feast-dev#4339](feast-dev#4339)) ([8e8c1f2](feast-dev@8e8c1f2))
* Bump psycopg2 to psycopg3 for all Postgres components ([feast-dev#4303](feast-dev#4303)) ([9451d9c](feast-dev@9451d9c))
* Entity key deserialization ([feast-dev#4284](feast-dev#4284)) ([83fad15](feast-dev@83fad15))
* Ignore paths feast apply ([feast-dev#4276](feast-dev#4276)) ([b4d54af](feast-dev@b4d54af))
* Move get_online_features to OnlineStore interface ([feast-dev#4319](feast-dev#4319)) ([7072fd0](feast-dev@7072fd0))
* Port mssql contrib offline store to ibis ([feast-dev#4360](feast-dev#4360)) ([7914cbd](feast-dev@7914cbd))

### Reverts

* Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([feast-dev#4357](feast-dev#4357)) ([cdeab48](feast-dev@cdeab48)), closes [feast-dev#4355](feast-dev#4355)
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.

Add get_online_features method in OnlineStore interface
2 participants