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: Removing the tls_verify_client flag from feast cli for offline server. #4842

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions sdk/python/feast/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1132,23 +1132,13 @@ def serve_registry_command(
show_default=False,
help="path to TLS certificate public key. You need to pass --key as well to start server in TLS mode",
)
@click.option(
"--verify_client",
"-v",
"tls_verify_client",
type=click.BOOL,
default="True",
show_default=True,
help="Verify the client or not for the TLS client certificate.",
)
@click.pass_context
def serve_offline_command(
ctx: click.Context,
host: str,
port: int,
tls_key_path: str,
tls_cert_path: str,
tls_verify_client: bool,
):
"""Start a remote server locally on a given host, port."""
if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path):
Expand All @@ -1157,7 +1147,7 @@ def serve_offline_command(
)
store = create_feature_store(ctx)

store.serve_offline(host, port, tls_key_path, tls_cert_path, tls_verify_client)
store.serve_offline(host, port, tls_key_path, tls_cert_path)


@cli.command("validate")
Expand Down
5 changes: 1 addition & 4 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -1964,14 +1964,11 @@ def serve_offline(
port: int,
tls_key_path: str = "",
tls_cert_path: str = "",
tls_verify_client: bool = True,
) -> None:
"""Start offline server locally on a given port."""
from feast import offline_server

offline_server.start_server(
self, host, port, tls_key_path, tls_cert_path, tls_verify_client
)
offline_server.start_server(self, host, port, tls_key_path, tls_cert_path)

def serve_transformations(self, port: int) -> None:
"""Start the feature transformation server locally on a given port."""
Expand Down
5 changes: 1 addition & 4 deletions sdk/python/feast/offline_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ def __init__(
location: str,
host: str = "localhost",
tls_certificates: List = [],
verify_client=False,
**kwargs,
):
super(OfflineServer, self).__init__(
Expand All @@ -54,7 +53,7 @@ def __init__(
str_to_auth_manager_type(store.config.auth_config.type)
),
tls_certificates=tls_certificates,
verify_client=verify_client,
verify_client=False, # this is needed for when we don't need mTLS
**kwargs,
)
self._location = location
Expand Down Expand Up @@ -568,7 +567,6 @@ def start_server(
port: int,
tls_key_path: str = "",
tls_cert_path: str = "",
tls_verify_client: bool = True,
):
_init_auth_manager(store)

Expand All @@ -591,7 +589,6 @@ def start_server(
location=location,
host=host,
tls_certificates=tls_certificates,
verify_client=tls_verify_client,
)
try:
logger.info(f"Offline store server serving at: {location}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,6 @@ def setup(self, registry: RegistryConfig):
str(tls_key_path),
"--cert",
str(self.tls_cert_path),
# This is needed for the self-signed certificate, disabled verify_client for integration tests.
"--verify_client",
str(False),
]
self.proc = subprocess.Popen(
cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL
Expand Down
Loading