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.

Sign up for GitHub

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

Jump to bottom

Allow use of secure gRPC in Feast Python client. #459

Merged
merged 3 commits into from
Feb 25, 2020
Merged

Allow use of secure gRPC in Feast Python client. #459

merged 3 commits into from
Feb 25, 2020

Conversation

Yanson
Copy link
Contributor

@Yanson Yanson commented Feb 4, 2020

What this PR does / why we need it:

Since #458 enables provisioning of an nginx ingress with TLS, when connecting from outside the cluster, secure gRPC is required.

Which issue(s) this PR fixes:

N/A

Does this PR introduce a user-facing change?:

If Feast Core or Serving gRPC APIs are listening on port 443, the Feast Client will default to enabling client-side TLS. If using non-secure gRPC on port 443, explicitly set the secure flag to false when constructing the client.

@feast-ci-bot
Copy link
Collaborator

Hi @Yanson. Thanks for your PR.

I'm waiting for a gojek member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Yanson
Copy link
Contributor Author

Yanson commented Feb 4, 2020

/assign @davidheryanto

@woop
Copy link
Member

woop commented Feb 5, 2020

Thanks for this @Yanson! Will try to have this reviewed soon.

@woop
Copy link
Member

woop commented Feb 11, 2020

/ok-to-test

@woop
Copy link
Member

woop commented Feb 11, 2020

@Yanson this PR looks good, but I think you will need to implement tests so that we know we aren't regressing in the future. Currently this is untested.

@Yanson
Copy link
Contributor Author

Yanson commented Feb 17, 2020

/retest

@zhilingc
Copy link
Collaborator

/approve

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Yanson, zhilingc

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

@woop
Copy link
Member

woop commented Feb 24, 2020

/lgtm

@feast-ci-bot feast-ci-bot removed the lgtm label Feb 25, 2020
@woop
Copy link
Member

woop commented Feb 25, 2020

/lgtm

@feast-ci-bot feast-ci-bot merged commit a576a53 into feast-dev:master Feb 25, 2020
davidheryanto pushed a commit to davidheryanto/feast that referenced this pull request Feb 26, 2020
@Yanson @davidheryanto
Allow use of secure gRPC in Feast Python client. (feast-dev#459)
c586d0e 
* Allow use of secure gRPC in Feast Python client.

* Add tests for secure gRPC in Python client.

(cherry picked from commit a576a53)
zhilingc pushed a commit that referenced this pull request Feb 26, 2020
@Yanson @zhilingc
Allow use of secure gRPC in Feast Python client. (#459)
eba0012 
* Allow use of secure gRPC in Feast Python client.

* Add tests for secure gRPC in Python client.

(cherry picked from commit a576a53)
@Yanson Yanson deleted the secure_grpc_client branch March 19, 2020 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidheryanto davidheryanto Awaiting requested review from davidheryanto

@khorshuheng khorshuheng Awaiting requested review from khorshuheng

@pradithya pradithya Awaiting requested review from pradithya

@woop woop Awaiting requested review from woop

@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@davidheryanto davidheryanto

@woop woop

Labels
approved lgtm ok-to-test size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Yanson @feast-ci-bot @woop @zhilingc @davidheryanto