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: Update RedisCluster to use redis-py official implementation #2554

Merged
merged 9 commits into from
Apr 15, 2022

Conversation

kevjumba
Copy link
Collaborator

@kevjumba kevjumba commented Apr 15, 2022

What this PR does / why we need it:
Update redis cluster implementation and remove redis cluster dependency. Also update crypto for mac m1 to 3.4.8(this is the newest version that patches some fixes in order to be compatible with openssl 1.1.1(the older version has been giving people with mac m1 trouble in installation).

Which issue(s) this PR fixes:

Fixes #2552

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
@kevjumba kevjumba changed the title fix: update RedisCluster to use redis-py official implementation fix: Update RedisCluster to use redis-py official implementation Apr 15, 2022
Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2022

Codecov Report

Merging #2554 (03aceb5) into master (f136f8c) will decrease coverage by 22.38%.
The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           master    #2554       +/-   ##
===========================================
- Coverage   81.55%   59.16%   -22.39%     
===========================================
  Files         141      140        -1     
  Lines       11936    11829      -107     
===========================================
- Hits         9734     6999     -2735     
- Misses       2202     4830     +2628     
Flag Coverage Δ
integrationtests ?
unittests 59.16% <50.00%> (+1.27%) ⬆️

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

Impacted Files Coverage Δ
sdk/python/feast/infra/online_stores/redis.py 28.39% <50.00%> (-67.91%) ⬇️
.../integration/online_store/test_online_retrieval.py 16.84% <0.00%> (-83.16%) ⬇️
sdk/python/tests/utils/online_read_write_test.py 19.35% <0.00%> (-80.65%) ⬇️
.../integration/online_store/test_universal_online.py 16.83% <0.00%> (-78.01%) ⬇️
...fline_store/test_universal_historical_retrieval.py 23.88% <0.00%> (-76.12%) ⬇️
...gration/registration/test_feature_service_apply.py 31.25% <0.00%> (-68.75%) ⬇️
sdk/python/tests/integration/e2e/test_usage_e2e.py 33.87% <0.00%> (-66.13%) ⬇️
...dk/python/tests/integration/e2e/test_validation.py 33.92% <0.00%> (-66.08%) ⬇️
sdk/python/tests/data/data_creator.py 34.78% <0.00%> (-65.22%) ⬇️
... and 73 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f136f8c...03aceb5. Read the comment docs.

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
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.

Single change

google-cloud-bigquery==2.34.1
# via feast (setup.py)
google-cloud-bigquery-storage==2.12.0
google-cloud-bigquery==3.0.1
Copy link
Member

Choose a reason for hiding this comment

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

We should pin this below 3.0.0. I think the 3+ is causing some issues, cc #2537 or #2532

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, let me do that right now.

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
Signed-off-by: Kevin Zhang <kzhang@tecton.ai>
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.

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, kevjumba

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

@feast-ci-bot feast-ci-bot merged commit ce5606f into feast-dev:master Apr 15, 2022
adchia pushed a commit that referenced this pull request Apr 20, 2022
* switch to using redis cluster in redis-py

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* fix

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Fix

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Fix dependencies

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Update py3.7-ci-requirements.txt

Signed-off-by: Felix Wang <wangfelix98@gmail.com>

* update 3.10 requirements

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Fix dependencies to address issues

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Fix

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

* Fix

Signed-off-by: Kevin Zhang <kzhang@tecton.ai>

Co-authored-by: Felix Wang <wangfelix98@gmail.com>
adchia pushed a commit that referenced this pull request Apr 20, 2022
## [0.20.1](v0.20.0...v0.20.1) (2022-04-20)

### Bug Fixes

* Addresses ZeroDivisionError when materializing file source with same timestamps ([#2551](#2551)) ([5539c51](5539c51))
* Build platform specific python packages with ci-build-wheel ([#2555](#2555)) ([1757639](1757639))
* Enforce kw args featureservice ([#2575](#2575)) ([4dce254](4dce254))
* Enforce kw args in datasources ([#2567](#2567)) ([6374634](6374634))
* Fix `__hash__` methods ([#2556](#2556)) ([dd8b854](dd8b854))
* Fix DynamoDB fetches when there are entities that are not found ([#2573](#2573)) ([882328f](882328f))
* Fix push sources and add docs / tests pushing via the python feature server ([#2561](#2561)) ([c5006c2](c5006c2))
* Fixed data mapping errors for Snowflake ([#2558](#2558)) ([abd6be7](abd6be7))
* Small typo in CLI ([#2578](#2578)) ([8717bc8](8717bc8))
* Switch from `join_key` to `join_keys` in tests and docs ([#2580](#2580)) ([6130b80](6130b80))
* Update build_go_protos to use a consistent python path ([#2550](#2550)) ([1c523bf](1c523bf))
* Update RedisCluster to use redis-py official implementation ([#2554](#2554)) ([c47fa2a](c47fa2a))
* Use cwd when getting module path ([#2577](#2577)) ([28752f2](28752f2))
achals pushed a commit that referenced this pull request May 13, 2022
# [0.21.0](v0.20.0...v0.21.0) (2022-05-13)

### Bug Fixes

* Addresses ZeroDivisionError when materializing file source with same timestamps ([#2551](#2551)) ([1e398d9](1e398d9))
* Asynchronously refresh registry for the feast ui command ([#2672](#2672)) ([1b09ca2](1b09ca2))
* Build platform specific python packages with ci-build-wheel ([#2555](#2555)) ([b10a4cf](b10a4cf))
* Delete data sources from registry when using the diffing logic ([#2669](#2669)) ([fc00ca8](fc00ca8))
* Enforce kw args featureservice ([#2575](#2575)) ([160d7b7](160d7b7))
* Enforce kw args in datasources ([#2567](#2567)) ([0b7ec53](0b7ec53))
* Feature logging to Redshift is broken ([#2655](#2655)) ([479cd51](479cd51))
* Feature service to templates ([#2649](#2649)) ([1e02066](1e02066))
* Feature with timestamp type is incorrectly interpreted by Go FS ([#2588](#2588)) ([e3d9588](e3d9588))
* Fix `__hash__` methods ([#2556](#2556)) ([ebb7dfe](ebb7dfe))
* Fix AWS bootstrap template ([#2604](#2604)) ([c94a69c](c94a69c))
* Fix broken proto conversion methods for data sources ([#2603](#2603)) ([00ed65a](00ed65a))
* Fix case where on demand feature view tab is broken if no custom tabs are passed.  ([#2682](#2682)) ([01d3568](01d3568))
* Fix DynamoDB fetches when there are entities that are not found ([#2573](#2573)) ([7076fe0](7076fe0))
* Fix Feast UI parser to work with new APIs ([#2668](#2668)) ([8d76751](8d76751))
* Fix java server after odfv update ([#2602](#2602)) ([0ca6297](0ca6297))
* Fix materialization with ttl=0 bug ([#2666](#2666)) ([ab78702](ab78702))
* Fix push sources and add docs / tests pushing via the python feature server ([#2561](#2561)) ([e8e418e](e8e418e))
* Fixed data mapping errors for Snowflake ([#2558](#2558)) ([53c2ce2](53c2ce2))
* Forcing ODFV udfs to be __main__ module and fixing false positive duplicate data source warning ([#2677](#2677)) ([2ce33cd](2ce33cd))
* Include the ui/build directory, and remove package data ([#2681](#2681)) ([0384f5f](0384f5f))
* Infer features for feature services when they depend on feature views without schemas ([#2653](#2653)) ([87c194c](87c194c))
* Pin dependencies to nearest major version ([#2647](#2647)) ([bb72b7c](bb72b7c))
* Pin pip<22.1 to get around breaking change in pip==22.1 ([#2678](#2678)) ([d3e01bc](d3e01bc))
* Punt deprecation warnings and clean up some warnings. ([#2670](#2670)) ([f775d2e](f775d2e))
* Reject undefined features when using `get_historical_features` or `get_online_features` ([#2665](#2665)) ([36849fb](36849fb))
* Remove ci extra from the feature transformation server dockerfile ([#2618](#2618)) ([25613b4](25613b4))
* Remove incorrect call to logging.basicConfig ([#2676](#2676)) ([8cbf51c](8cbf51c))
* Small typo in CLI ([#2578](#2578)) ([f372981](f372981))
* Switch from `join_key` to `join_keys` in tests and docs ([#2580](#2580)) ([d66c931](d66c931))
* Teardown trino container correctly after tests ([#2562](#2562)) ([72f1558](72f1558))
* Update build_go_protos to use a consistent python path ([#2550](#2550)) ([f136f8c](f136f8c))
* Update data source timestamp inference error message to make sense ([#2636](#2636)) ([3eaf6b7](3eaf6b7))
* Update field api to add tag parameter corresponding to labels in Feature. ([#2610](#2610)) ([689d20b](689d20b))
* Update java integration tests and add more logging ([#2637](#2637)) ([10e23b4](10e23b4))
* Update on demand feature view api ([#2587](#2587)) ([38cd7f9](38cd7f9))
* Update RedisCluster to use redis-py official implementation ([#2554](#2554)) ([ce5606f](ce5606f))
* Use cwd when getting module path ([#2577](#2577)) ([b550e59](b550e59))
* Use ParquetDataset for Schema Inference ([#2686](#2686)) ([4f85e3e](4f85e3e))
* Use timestamp type when converting unixtimestamp feature type to arrow ([#2593](#2593)) ([c439611](c439611))

### Features

* Add hbase online store support in feast ([#2590](#2590)) ([c9eda79](c9eda79))
* Adding SSL options for Postgres ([#2644](#2644)) ([0e809c2](0e809c2))
* Allow Feast UI to be spun up with CLI command: feast ui ([#2667](#2667)) ([44ca9f5](44ca9f5))
* Allow to pass secrets and environment variables to transformation service ([#2632](#2632)) ([ffa33ad](ffa33ad))
* CLI command 'feast serve' should start go-based server if flag is enabled ([#2617](#2617)) ([f3ff812](f3ff812))
* Create stream and batch feature view abstractions ([#2559](#2559)) ([d1f76e5](d1f76e5))
* Postgres supported as Registry, Online store, and Offline store ([#2401](#2401)) ([ed2f979](ed2f979))
* Support entity fields in feature view `schema` parameter by dropping them ([#2568](#2568)) ([c8fcc35](c8fcc35))
* Write logged features to an offline store (Python API) ([#2574](#2574)) ([134dc5f](134dc5f))
* Write logged features to Offline Store (Go - Python integration) ([#2621](#2621)) ([ccad832](ccad832))

### Reverts

* Revert "chore: Deprecate value type (#2611)" (#2643) ([4fbdfb1](4fbdfb1)), closes [#2611](#2611) [#2643](#2643)
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.

Switch from redis-py-cluster to redis-py
5 participants