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: Add mysql as online store #3190

Merged
merged 17 commits into from Sep 22, 2022

Conversation

hao-affirm
Copy link
Contributor

@hao-affirm hao-affirm commented Sep 7, 2022

Signed-off-by: hao-affirm 104030690+hao-affirm@users.noreply.github.com

What this PR does / why we need it:
Add mysql as a online store

Which issue(s) this PR fixes:

Fixes #

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
@feast-ci-bot feast-ci-bot added size/L and removed size/S labels Sep 7, 2022
@hao-affirm hao-affirm changed the title Add mysql as online store feast: Add mysql as online store Sep 7, 2022
@hao-affirm hao-affirm changed the title feast: Add mysql as online store feat: Add mysql as online store Sep 7, 2022
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2022

Codecov Report

Base: 67.03% // Head: 75.48% // Increases project coverage by +8.44% 🎉

Coverage data is based on head (0d7d831) compared to base (b4ef834).
Patch coverage: 39.81% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3190      +/-   ##
==========================================
+ Coverage   67.03%   75.48%   +8.44%     
==========================================
  Files         175      214      +39     
  Lines       15941    18031    +2090     
==========================================
+ Hits        10686    13610    +2924     
+ Misses       5255     4421     -834     
Flag Coverage Δ
integrationtests 66.85% <ø> (-0.19%) ⬇️
unittests 57.79% <39.81%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdk/python/feast/repo_config.py 87.83% <ø> (+5.32%) ⬆️
setup.py 0.00% <ø> (ø)
.../online_stores/contrib/mysql_online_store/mysql.py 35.86% <35.86%> (ø)
...tion/feature_repos/universal/online_store/mysql.py 53.84% <53.84%> (ø)
.../online_stores/contrib/mysql_repo_configuration.py 100.00% <100.00%> (ø)
...on/feast/infra/materialization/snowflake_engine.py 92.09% <0.00%> (-0.51%) ⬇️
sdk/python/feast/infra/online_stores/snowflake.py 98.16% <0.00%> (-0.02%) ⬇️
sdk/python/feast/infra/provider.py 77.94% <0.00%> (ø)
sdk/python/feast/infra/passthrough_provider.py 96.29% <0.00%> (ø)
sdk/python/feast/infra/offline_stores/snowflake.py 93.30% <0.00%> (ø)
... and 109 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
@felixwang9817 felixwang9817 self-assigned this Sep 8, 2022
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.

looks pretty good overall!

lint errors need to be fixed

also, if you want this online store contribution to be more public, you should feel free to add some docs for it as the guide suggests! specifically you can add docs to Gitbook by following the steps here and you can add docs to RTD in sdk/python/docs/index.rst (the structure should be pretty self-explanatory, and you can check the results with make html)

sdk/python/feast/infra/online_stores/contrib/mysql.py Outdated Show resolved Hide resolved
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Copy link
Member

@achals achals left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks for the contribution @hao-affirm !! I've left some questions/comments but it looks mostly great.

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
@hao-affirm hao-affirm requested review from achals and removed request for felixwang9817 September 10, 2022 02:34
@hao-affirm
Copy link
Contributor Author

@felixwang9817 @achals any reason

self.config.online_store and self.config.online_store.type == "sqlite"
that we can not use other db like mysql?

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

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: felixwang9817, hao-affirm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@felixwang9817
Copy link
Collaborator

hey @hao-affirm sorry for the delay on the review here - I just stamped it!

in terms of enabling plan for mysql, there's no inherent reason why it can't be done - it should be a reasonably easy change to make, but I think it should come in a follow up PR

@feast-ci-bot feast-ci-bot merged commit cb8db84 into feast-dev:master Sep 22, 2022
felixwang9817 pushed a commit to felixwang9817/feast that referenced this pull request Sep 26, 2022
* update remove debug

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* add interface

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update sql

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update ci reqs

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* remove pip index

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix unit test issue

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint issue

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* add entity_key_serialization_version

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update doc

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix makefile typo

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* move to func

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
vmallya-123 pushed a commit to vmallya-123/feast that referenced this pull request Sep 27, 2022
* update remove debug

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* add interface

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update sql

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update ci reqs

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* remove pip index

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix unit test issue

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix lint issue

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* add entity_key_serialization_version

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* update doc

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* fix makefile typo

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* move to func

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

* format

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>

Signed-off-by: hao-affirm <104030690+hao-affirm@users.noreply.github.com>
kevjumba pushed a commit that referenced this pull request Oct 6, 2022
# [0.26.0](v0.25.0...v0.26.0) (2022-10-06)

### Bug Fixes

* Add `X-Trino-Extra-Credential` header and remove user override ([#3246](#3246)) ([164e666](164e666))
* Add postgres to the feature server Dockerfile to fix helm chart flow ([#3261](#3261)) ([6f6cbb7](6f6cbb7))
* Add stream feature view in the Web UI ([#3257](#3257)) ([1f70b3a](1f70b3a))
* Build dockerfile correctly ([#3239](#3239)) ([a2dc0d0](a2dc0d0))
* Configuration to stop coercion of tz for entity_df ([#3255](#3255)) ([97b7ab9](97b7ab9))
* Enable users to upgrade a batch source into a push source ([#3213](#3213)) ([1b312fb](1b312fb))
* Fix docker image for feature-server ([#3272](#3272)) ([eff01d1](eff01d1))
* Fix Feast UI release process to update the feast-ui package  ([#3267](#3267)) ([a9d48d0](a9d48d0))
* Return 422 on bad push source name ([#3214](#3214)) ([b851e01](b851e01))
* Stream feature view meta undefined created_timestamp issue ([#3266](#3266)) ([12e1a8f](12e1a8f))
* Stream feature view not shown in the UI ([#3251](#3251)) ([e713dda](e713dda))
* Udf in stream feature view UI shows pickled data ([#3268](#3268)) ([0728117](0728117))
* Update snowflake materialization messages ([#3230](#3230)) ([a63d440](a63d440))
* Updated quickstart notebook to patch an incorrect reference to an outdated featureview name ([#3271](#3271)) ([b9b9c54](b9b9c54))
* Use configured user in env var instead of "user" for Trino ([#3254](#3254)) ([532d8a1](532d8a1))

### Features

* Add mysql as online store ([#3190](#3190)) ([cb8db84](cb8db84))
* Add possibility to define feature_store.yaml path with env variable ([#3231](#3231)) ([95fdb19](95fdb19))
* Add request_timeout setting for cassandra online store adapter ([#3256](#3256)) ([da20757](da20757))
* Add tag description in Field in the Feast UI ([#3258](#3258)) ([086f279](086f279))
* Adding billing_project_id in BigQueryOfflineStoreConfig ([#3253](#3253)) ([f80f05f](f80f05f))
* BigTable online store ([#3140](#3140)) ([6bc91c2](6bc91c2)), closes [/github.com/spulec/moto/blob/master/CHANGELOG.md#400](https://github.com//github.com/spulec/moto/blob/master/CHANGELOG.md/issues/400)
* Filter subset features in postgres [#3174](#3174) ([#3203](#3203)) ([b48d36b](b48d36b))
franciscojavierarceo pushed a commit to franciscojavierarceo/feast that referenced this pull request Oct 18, 2022
# [0.26.0](feast-dev/feast@v0.25.0...v0.26.0) (2022-10-06)

### Bug Fixes

* Add `X-Trino-Extra-Credential` header and remove user override ([feast-dev#3246](feast-dev#3246)) ([164e666](feast-dev@164e666))
* Add postgres to the feature server Dockerfile to fix helm chart flow ([feast-dev#3261](feast-dev#3261)) ([6f6cbb7](feast-dev@6f6cbb7))
* Add stream feature view in the Web UI ([feast-dev#3257](feast-dev#3257)) ([1f70b3a](feast-dev@1f70b3a))
* Build dockerfile correctly ([feast-dev#3239](feast-dev#3239)) ([a2dc0d0](feast-dev@a2dc0d0))
* Configuration to stop coercion of tz for entity_df ([feast-dev#3255](feast-dev#3255)) ([97b7ab9](feast-dev@97b7ab9))
* Enable users to upgrade a batch source into a push source ([feast-dev#3213](feast-dev#3213)) ([1b312fb](feast-dev@1b312fb))
* Fix docker image for feature-server ([feast-dev#3272](feast-dev#3272)) ([eff01d1](feast-dev@eff01d1))
* Fix Feast UI release process to update the feast-ui package  ([feast-dev#3267](feast-dev#3267)) ([a9d48d0](feast-dev@a9d48d0))
* Return 422 on bad push source name ([feast-dev#3214](feast-dev#3214)) ([b851e01](feast-dev@b851e01))
* Stream feature view meta undefined created_timestamp issue ([feast-dev#3266](feast-dev#3266)) ([12e1a8f](feast-dev@12e1a8f))
* Stream feature view not shown in the UI ([feast-dev#3251](feast-dev#3251)) ([e713dda](feast-dev@e713dda))
* Udf in stream feature view UI shows pickled data ([feast-dev#3268](feast-dev#3268)) ([0728117](feast-dev@0728117))
* Update snowflake materialization messages ([feast-dev#3230](feast-dev#3230)) ([a63d440](feast-dev@a63d440))
* Updated quickstart notebook to patch an incorrect reference to an outdated featureview name ([feast-dev#3271](feast-dev#3271)) ([b9b9c54](feast-dev@b9b9c54))
* Use configured user in env var instead of "user" for Trino ([feast-dev#3254](feast-dev#3254)) ([532d8a1](feast-dev@532d8a1))

### Features

* Add mysql as online store ([feast-dev#3190](feast-dev#3190)) ([cb8db84](feast-dev@cb8db84))
* Add possibility to define feature_store.yaml path with env variable ([feast-dev#3231](feast-dev#3231)) ([95fdb19](feast-dev@95fdb19))
* Add request_timeout setting for cassandra online store adapter ([feast-dev#3256](feast-dev#3256)) ([da20757](feast-dev@da20757))
* Add tag description in Field in the Feast UI ([feast-dev#3258](feast-dev#3258)) ([086f279](feast-dev@086f279))
* Adding billing_project_id in BigQueryOfflineStoreConfig ([feast-dev#3253](feast-dev#3253)) ([f80f05f](feast-dev@f80f05f))
* BigTable online store ([feast-dev#3140](feast-dev#3140)) ([6bc91c2](feast-dev@6bc91c2)), closes [/github.com/spulec/moto/blob/master/CHANGELOG.md#400](https://github.com//github.com/spulec/moto/blob/master/CHANGELOG.md/issues/400)
* Filter subset features in postgres [feast-dev#3174](feast-dev#3174) ([feast-dev#3203](feast-dev#3203)) ([b48d36b](feast-dev@b48d36b))
@IsraelAgundis IsraelAgundis deleted the mysql-online-store branch September 1, 2023 20:03
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.

None yet

6 participants