Skip to content

feat: Allowing password based authentication and SSL for Redis in Java feature server#2322

Merged
feast-ci-bot merged 7 commits intofeast-dev:masterfrom
adchia:redis-auth
Feb 23, 2022
Merged

feat: Allowing password based authentication and SSL for Redis in Java feature server#2322
feast-ci-bot merged 7 commits intofeast-dev:masterfrom
adchia:redis-auth

Conversation

@adchia
Copy link
Copy Markdown
Collaborator

@adchia adchia commented Feb 18, 2022

What this PR does / why we need it:
This is a partial forward port of feast-dev/feast-java-old#44 to enable auth for Redis in the Java feature server (Python already handles this)

Which issue(s) this PR fixes:

Fixes # #2285

Does this PR introduce a user-facing change?:

Java feature server now supports SSL and password based authentication for Redis

@adchia
Enabling password authentication and SSL in Redis clusters for the Ja…
429db38 
…va feature server

Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia
lint
f150646 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia
add test
08dde62 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia adchia requested a review from a team as a code owner February 18, 2022 03:15
@adchia adchia requested review from tsotnet and removed request for a team February 18, 2022 03:15
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 18, 2022 •
edited
Loading

Codecov Report

Merging #2322 (83182f1) into master (876986f) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2322      +/-   ##
==========================================
+ Coverage   58.73%   58.76%   +0.02%     
==========================================
  Files         116      116              
  Lines        9888     9894       +6     
==========================================
+ Hits         5808     5814       +6     
  Misses       4080     4080
Flag Coverage Δ
unittests 58.76% <ø> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
sdk/python/tests/conftest.py 59.40% <0.00%> (ø)
sdk/python/feast/feature_view_projection.py 69.23% <0.00%> (ø)
...ts/integration/feature_repos/repo_configuration.py 44.95% <0.00%> (ø)
sdk/python/feast/feature_service.py 71.29% <0.00%> (+1.68%) ⬆️

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 876986f...83182f1. Read the comment docs.

andrijaperovic
andrijaperovic reviewed Feb 18, 2022
View reviewed changes
Copy link
Copy Markdown

@andrijaperovic andrijaperovic left a comment

Choose a reason for hiding this comment

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

@adchia do you plan to port the tcp keepalive and socket resolver changes as well? Or you want me to add these changes separately.

@adchia
Copy link
Copy Markdown
Collaborator Author

adchia commented Feb 18, 2022

@adchia do you plan to port the tcp keepalive and socket resolver changes as well? Or you want me to add these changes separately.

@andrijaperovic yeah can you make the other changes too? Wanted to get this one in first since it's blocking some users

@adchia
Address comments
01afe5b 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia
Address comments
9465882 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia
Copy link
Copy Markdown
Collaborator Author

adchia commented Feb 22, 2022

friendly bump @woop

@adchia
lint
9b0f470 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia
lint
83182f1 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia adchia changed the title Allowing password based authentication and SSL for Redis in Java feature server feat: Allowing password based authentication and SSL for Redis in Java feature server Feb 23, 2022
@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Feb 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, woop

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

The pull request process is described here

Details 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

@woop
Copy link
Copy Markdown
Member

woop commented Feb 23, 2022

/lgtm

@woop woop added the kind/feature New feature or request label Feb 23, 2022
@feast-ci-bot feast-ci-bot merged commit 0af8adb into feast-dev:master Feb 23, 2022
@adchia adchia deleted the redis-auth branch February 23, 2022 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop approved these changes

@andrijaperovic andrijaperovic andrijaperovic left review comments

@tsotnet tsotnet Awaiting requested review from tsotnet tsotnet was automatically assigned from feast-dev/maintainers

Assignees

@andrijaperovic andrijaperovic

@tsotnet tsotnet

@woop woop

Labels

approved kind/feature New feature or request lgtm release-note size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@adchia @codecov-commenter @feast-ci-bot @woop @andrijaperovic @tsotnet