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

Feature/SK-948 | Combiner config flags in fedn client #709

Merged
merged 3 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
29 changes: 22 additions & 7 deletions fedn/cli/client_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
import click
import requests

from fedn.common.exceptions import InvalidClientConfig
from fedn.network.clients.client import Client

from fedn.cli.main import main
from fedn.cli.shared import CONTROLLER_DEFAULTS, apply_config, get_api_url, get_token, print_response
from fedn.common.exceptions import InvalidClientConfig
from fedn.network.clients.client import Client


def validate_client_config(config):
Expand All @@ -17,18 +16,20 @@ def validate_client_config(config):
"""
try:
if config["discover_host"] is None or config["discover_host"] == "":
raise InvalidClientConfig("Missing required configuration: discover_host")
if config["combiner"] is None or config["combiner"] == "":
raise InvalidClientConfig("Missing required configuration: discover_host or combiner")
if "discover_port" not in config.keys():
config["discover_port"] = None
if config["remote_compute_context"] and config["discover_host"] is None:
raise InvalidClientConfig("Remote compute context requires discover_host")
except Exception:
raise InvalidClientConfig("Could not load config from file. Check config")


@main.group("client")
@click.pass_context
def client_cmd(ctx):
""":param ctx:
"""
""":param ctx:"""
pass


Expand Down Expand Up @@ -79,7 +80,10 @@ def list_clients(ctx, protocol: str, host: str, port: str, token: str = None, n_
@click.option("-s", "--secure", required=False, default=False)
@click.option("-pc", "--preshared-cert", required=False, default=False)
@click.option("-v", "--verify", is_flag=True, help="Verify SSL/TLS for REST service")
@click.option("-c", "--preferred-combiner", type=str,required=False, default="",help="name of the preferred combiner")
@click.option("-c", "--preferred-combiner", type=str, required=False, default="", help="name of the preferred combiner")
@click.option("--combiner", type=str, required=False, default="", help="Skip combiner assignment from discover service and attatch directly to combiner host.")
@click.option("--combiner-port", type=str, required=False, default="12080", help="Combiner port, need to be used with --combiner")
@click.option("--proxy-server", type=str, required=False, default="", help="gRPC proxy server, need to be used together with --combiner")
@click.option("-va", "--validator", required=False, default=True)
@click.option("-tr", "--trainer", required=False, default=True)
@click.option("-in", "--init", required=False, default=None, help="Set to a filename to (re)init client from file state.")
Expand All @@ -102,6 +106,9 @@ def client_cmd(
preshared_cert,
verify,
preferred_combiner,
combiner,
combiner_port,
proxy_server,
validator,
trainer,
init,
Expand Down Expand Up @@ -143,6 +150,9 @@ def client_cmd(
"preshared_cert": preshared_cert,
"verify": verify,
"preferred_combiner": preferred_combiner,
"combiner": combiner,
"combiner_port": combiner_port,
"proxy_server": proxy_server,
"validator": validator,
"trainer": trainer,
"logfile": logfile,
Expand All @@ -156,6 +166,11 @@ def client_cmd(
click.echo(f"\nClient configuration loaded from file: {init}")
click.echo("Values set in file override defaults and command line arguments...\n")

# proxy_server needs combiner check
if config["proxy_server"]:
if not config["combiner"]:
click.echo("--proxy-server/proxy_server requires a combiner host in --combiner/combiner")
return
try:
validate_client_config(config)
except InvalidClientConfig as e:
Expand Down
21 changes: 7 additions & 14 deletions fedn/network/clients/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,6 @@ def __init__(self, config):

self.id = config["client_id"] or str(uuid.uuid4())

self.connector = ConnectorClient(
host=config["discover_host"],
port=config["discover_port"],
token=config["token"],
name=config["name"],
remote_package=config["remote_compute_context"],
force_ssl=config["force_ssl"],
verify=config["verify"],
combiner=config["preferred_combiner"],
id=self.id,
)

# Validate client name
match = re.search(VALID_NAME_REGEX, config["name"])
if not match:
Expand All @@ -94,8 +82,13 @@ def __init__(self, config):

self.inbox = queue.Queue()

# Attach to the FEDn network (get combiner)
combiner_config = self.assign()
# Attach to the FEDn network (get combiner or attach directly)
if config["combiner"]:
combiner_config = {"status": "assigned", "host": config["combiner"], "port": config["combiner_port"], "helper_type": ""}
if config["proxy_server"]:
combiner_config["fqdn"] = config["proxy_server"]
else:
combiner_config = self.assign()
self.connect(combiner_config)

self._initialize_dispatcher(self.config)
Expand Down
Loading