-
Notifications
You must be signed in to change notification settings - Fork 1k
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 Hazelcast as an online store #3523
Conversation
Signed-off-by: mehmettokgoz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good - going to do one more pass over when I have some bandwidth!
ssl_cafile_path: /path/to/ca/file | ||
ssl_certfile_path: /path/to/cert/file | ||
ssl_keyfile_path: /path/to/key/file | ||
ssl_password: <YOUR_SSL_PASSWORD> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, this should be read from the enrvironment instead of being hardcoded in the feature_store.yaml
file.
ssl_password: <YOUR_SSL_PASSWORD> | |
ssl_password: ${SSL_PASSWORD} # This value will be read form the `SSL_PASSWORD` environment variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, thanks! Replaced the SSL password with environment variable at dad8fc4.
ssl_cafile_path: /path/to/ca/file | ||
ssl_certfile_path: /path/to/cert/file | ||
ssl_keyfile_path: /path/to/key/file | ||
ssl_password: <YOUR_SSL_PASSWORD> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
for entity_key, values, event_ts, created_ts in data: | ||
entity_key_str = base64.b64encode( | ||
serialize_entity_key( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For new online stores, we should just be using entity_key_serialization_version=2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced the config parameter with version 2 at dad8fc4.
entity_keys_str = {} | ||
for entity_key in entity_keys: | ||
entity_key_str = base64.b64encode( | ||
serialize_entity_key( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test |
Signed-off-by: mehmettokgoz <[email protected]>
Hey @achals 👋 I've updated the PR and addressed your review comments. Let me know if you need my help! I've also tested manually using the template and the test workflow runs correctly. |
Signed-off-by: mehmettokgoz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, mehmettokgoz 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 |
# [0.31.0](v0.30.0...v0.31.0) (2023-04-21) ### Bug Fixes * Add Stream Feature Views to helper that collect Feature View names ([#3582](#3582)) ([7854f63](7854f63)) * Add StreamFeatureViewSpec to FeastObjectSpecProto convenience type ([#3550](#3550)) ([3cefd6c](3cefd6c)) * Batch Snowflake materialization queries to obey Snowpark 100 fea… ([#3406](#3406)) ([f9862b5](f9862b5)) * Bytewax materializer security context ([#3573](#3573)) ([6794338](6794338)) * **cI:** Install coreutils in mac github workers for smoke test ([#3563](#3563)) ([e7421c1](e7421c1)) * Fix bug with no SqlRegistryConfig class ([#3586](#3586)) ([6dc1368](6dc1368)) * Fix Snowflake template ([#3584](#3584)) ([6c09c39](6c09c39)) * Make snowflake to remote tables temporary ([#3588](#3588)) ([ad48146](ad48146)) * Remove snowflake source warehouse tech debt ([#3422](#3422)) ([7da0580](7da0580)) * Snowflake remote storage ([#3574](#3574)) ([f8d3890](f8d3890)) * Support param timeout when persisting ([#3593](#3593)) ([01a98f0](01a98f0)) * Use pyarrow in a way that works across versions ([#3562](#3562)) ([1289f3f](1289f3f)) * Wrap the bigquery table name with backtick. ([#3577](#3577)) ([09f0e7e](09f0e7e)) ### Features * Add AWS Redshift Serverless support ([#3595](#3595)) ([58ce148](58ce148)) * Add Hazelcast as an online store ([#3523](#3523)) ([b05d50b](b05d50b)) * Cache Bigtable client ([#3602](#3602)) ([b27472f](b27472f)) * Relax aws extras requirements ([#3585](#3585)) ([7e77382](7e77382)) * Show bigquery datasource table and query on UI ([#3600](#3600)) ([58d63f7](58d63f7)) * Update snowflake offline store job output formats -- added arrow ([#3589](#3589)) ([be3e349](be3e349))
# [0.31.0](feast-dev/feast@v0.30.0...v0.31.0) (2023-04-21) ### Bug Fixes * Add Stream Feature Views to helper that collect Feature View names ([feast-dev#3582](feast-dev#3582)) ([7854f63](feast-dev@7854f63)) * Add StreamFeatureViewSpec to FeastObjectSpecProto convenience type ([feast-dev#3550](feast-dev#3550)) ([3cefd6c](feast-dev@3cefd6c)) * Batch Snowflake materialization queries to obey Snowpark 100 fea… ([feast-dev#3406](feast-dev#3406)) ([f9862b5](feast-dev@f9862b5)) * Bytewax materializer security context ([feast-dev#3573](feast-dev#3573)) ([6794338](feast-dev@6794338)) * **cI:** Install coreutils in mac github workers for smoke test ([feast-dev#3563](feast-dev#3563)) ([e7421c1](feast-dev@e7421c1)) * Fix bug with no SqlRegistryConfig class ([feast-dev#3586](feast-dev#3586)) ([6dc1368](feast-dev@6dc1368)) * Fix Snowflake template ([feast-dev#3584](feast-dev#3584)) ([6c09c39](feast-dev@6c09c39)) * Make snowflake to remote tables temporary ([feast-dev#3588](feast-dev#3588)) ([ad48146](feast-dev@ad48146)) * Remove snowflake source warehouse tech debt ([feast-dev#3422](feast-dev#3422)) ([7da0580](feast-dev@7da0580)) * Snowflake remote storage ([feast-dev#3574](feast-dev#3574)) ([f8d3890](feast-dev@f8d3890)) * Support param timeout when persisting ([feast-dev#3593](feast-dev#3593)) ([01a98f0](feast-dev@01a98f0)) * Use pyarrow in a way that works across versions ([feast-dev#3562](feast-dev#3562)) ([1289f3f](feast-dev@1289f3f)) * Wrap the bigquery table name with backtick. ([feast-dev#3577](feast-dev#3577)) ([09f0e7e](feast-dev@09f0e7e)) ### Features * Add AWS Redshift Serverless support ([feast-dev#3595](feast-dev#3595)) ([58ce148](feast-dev@58ce148)) * Add Hazelcast as an online store ([feast-dev#3523](feast-dev#3523)) ([b05d50b](feast-dev@b05d50b)) * Cache Bigtable client ([feast-dev#3602](feast-dev#3602)) ([b27472f](feast-dev@b27472f)) * Relax aws extras requirements ([feast-dev#3585](feast-dev#3585)) ([7e77382](feast-dev@7e77382)) * Show bigquery datasource table and query on UI ([feast-dev#3600](feast-dev#3600)) ([58d63f7](feast-dev@58d63f7)) * Update snowflake offline store job output formats -- added arrow ([feast-dev#3589](feast-dev#3589)) ([be3e349](feast-dev@be3e349)) Signed-off-by: zerafachris PERSONAL <[email protected]>
What this PR does / why we need it:
This PR adds Hazelcast as an online store. The implementation cover the
online_write_batch
,online_read
,teardown
andupdate
methods and adds interactive CLI to initialize Hazelcast template.Signed-off by: Mehmet Tokgöz [email protected]
Which issue(s) this PR fixes:
Fixes #
No related issue.