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

Fix/SK-1127 | add session_id flag #742

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Fix/SK-1127 | add session_id flag #742

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
035f652
session_id flag added
Nov 8, 2024
131e045
minor code fix
Nov 11, 2024
ef2543d
added new line end of session cmd file
Nov 11, 2024
794a0b1
fixed acc to feedback
Nov 13, 2024
c83e928
code fix
Nov 13, 2024
7c24f1a
fixed conflict
Nov 13, 2024
4a1d035
conflict + minor fix
Nov 13, 2024
282c81b
spelling fix:)
Nov 13, 2024
2500829
resolving conflict
Nov 13, 2024
f285468
trying to add get_client function again
Nov 13, 2024
581cfbc
header prints removed
Nov 15, 2024
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
43 changes: 35 additions & 8 deletions fedn/cli/client_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ def client_cmd(ctx):
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Client ID")
@click.option("--n_max", required=False, help="Number of items to list")
@client_cmd.command("list")
@click.pass_context
def list_clients(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None, n_max: int = None):
def list_clients(ctx, protocol: str, host: str, port: str, token: str = None, n_max: int = None):
"""Return:
------
- count: number of clients
Expand All @@ -58,6 +57,38 @@ def list_clients(ctx, protocol: str, host: str, port: str, token: str = None, id

_token = get_token(token)

if _token:
headers["Authorization"] = _token


click.echo(f"\nListing clients: {url}\n")
click.echo(f"Headers: {headers}")
try:
response = requests.get(url, headers=headers)
print_response(response, "clients", None)
except requests.exceptions.ConnectionError:
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 handle more than just ConnectionError, there are plenty of other error related to http requests. You don't have do do this know but make an other issue of it. For example create a utility function that handles all requests and checks the response for error:

import requests
from requests.exceptions import HTTPError, Timeout, ConnectionError, RequestException

def make_request(method, url, **kwargs):
"""
A reusable function to handle HTTP(S) requests with error handling.

Args:
    method (str): HTTP method (e.g., 'GET', 'POST').
    url (str): The URL for the request.
    **kwargs: Additional arguments passed to `requests.request()`.

Returns:
    dict: A dictionary with the response data or an error message.
"""
try:
    response = requests.request(method, url, **kwargs)
    response.raise_for_status()
    # Assuming JSON response, adjust if needed
    return {"success": True, "data": response.json()}
except HTTPError as http_err:
    return {"success": False, "error": f"HTTP error occurred: {http_err}"}
except Timeout:
    return {"success": False, "error": "The request timed out"}
except ConnectionError:
    return {"success": False, "error": "Connection error occurred"}
except RequestException as req_err:
    return {"success": False, "error": f"Request error: {req_err}"}
except Exception as e:
    return {"success": False, "error": f"Unexpected error: {e}"}

click.echo(f"Error: Could not connect to {url}")


@click.option("-p", "--protocol", required=False, default=CONTROLLER_DEFAULTS["protocol"], help="Communication protocol of controller (api)")
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Client ID")
@client_cmd.command("get")
@click.pass_context
def get_client(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Think id should be required...

"""Return:
------
- count: number of clients
- result: list of clients

"""
url = get_api_url(protocol=protocol, host=host, port=port, endpoint="clients")
headers = {}

_token = get_token(token)

if _token:
headers["Authorization"] = _token

Expand All @@ -66,18 +97,14 @@ def list_clients(ctx, protocol: str, host: str, port: str, token: str = None, id
headers["id"] = id
Copy link
Contributor

Choose a reason for hiding this comment

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

This header should not be needed.



click.echo(f"\nListing clients: {url}\n")
click.echo(f"\nRetrieving client: {url}\n")
click.echo(f"Headers: {headers}")
Copy link
Member

Choose a reason for hiding this comment

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

remove print of hearder

try:
response = requests.get(url, headers=headers)
if id:
print_response(response, "client", True)
else:
print_response(response, "clients", False)
print_response(response, "client", id)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")


@client_cmd.command("start")
@click.option("-d", "--discoverhost", required=False, help="Hostname for discovery services(reducer).")
@click.option("-p", "--discoverport", required=False, help="Port for discovery services (reducer).")
Expand Down
42 changes: 35 additions & 7 deletions fedn/cli/combiner_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,10 @@ def start_cmd(ctx, discoverhost, discoverport, token, name, host, port, fqdn, se
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Combiner ID")
@click.option("--n_max", required=False, help="Number of items to list")
@combiner_cmd.command("list")
@click.pass_context
def list_combiners(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None, n_max: int = None):
def list_combiners(ctx, protocol: str, host: str, port: str, token: str = None, n_max: int = None):
"""Return:
------
- count: number of combiners
Expand All @@ -84,6 +83,38 @@ def list_combiners(ctx, protocol: str, host: str, port: str, token: str = None,
if n_max:
headers["X-Limit"] = n_max

_token = get_token(token)

if _token:
headers["Authorization"] = _token


click.echo(f"\nListing combiners: {url}\n")
click.echo(f"Headers: {headers}")
try:
response = requests.get(url, headers=headers)
print_response(response, "combiners", None)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")


@click.option("-p", "--protocol", required=False, default=CONTROLLER_DEFAULTS["protocol"], help="Communication protocol of controller (api)")
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Combiner ID")
@combiner_cmd.command("get")
@click.pass_context
def get_combiner(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None):
"""Return:
------
- result: combiner with given id

"""
url = get_api_url(protocol=protocol, host=host, port=port, endpoint="combiners")
headers = {}


_token = get_token(token)

if _token:
Expand All @@ -94,13 +125,10 @@ def list_combiners(ctx, protocol: str, host: str, port: str, token: str = None,
headers["id"] = id


click.echo(f"\nListing combiners: {url}\n")
click.echo(f"\nRetrieving combiner: {url}\n")
click.echo(f"Headers: {headers}")
Copy link
Member

Choose a reason for hiding this comment

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

remove

try:
response = requests.get(url, headers=headers)
if id:
print_response(response, "combiner", True)
else:
print_response(response, "combiners", False)
print_response(response, "combiner", id)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")
68 changes: 42 additions & 26 deletions fedn/cli/model_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,62 @@ def model_cmd(ctx):
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Model ID")
@click.option("-session_id", "--session_id", required=False, help="models in session with given session id")
@click.option("--n_max", required=False, help="Number of items to list")
@model_cmd.command("list")
@click.pass_context
def list_models(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None, session_id: str = None, n_max: int = None):
def list_models(ctx, protocol: str, host: str, port: str, token: str = None, session_id: str = None, n_max: int = None):
"""Return:
------
- count: number of models
- result: list of models

"""
url = get_api_url(protocol=protocol, host=host, port=port, endpoint="models")


headers = {}

if n_max:
headers["X-Limit"] = n_max

_token = get_token(token)

if _token:
headers["Authorization"] = _token

if session_id:
url = f"{url}?session_id={session_id}"
headers["session_id"] = session_id
Copy link
Contributor

Choose a reason for hiding this comment

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

This header should not be needed.


click.echo(f"\nListing models: {url}\n")
click.echo(f"Headers: {headers}")
try:
response = requests.get(url, headers=headers)
print_response(response, "models", None)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")


@click.option("-p", "--protocol", required=False, default=CONTROLLER_DEFAULTS["protocol"], help="Communication protocol of controller (api)")
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Model ID")
@model_cmd.command("get")
@click.pass_context
def get_model(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None):
"""Return:
------
- result: model with given id

"""
url = get_api_url(protocol=protocol, host=host, port=port, endpoint="models")


headers = {}


_token = get_token(token)

if _token:
Expand All @@ -44,32 +82,10 @@ def list_models(ctx, protocol: str, host: str, port: str, token: str = None, id:
url = f"{url}{id}"
headers["id"] = id


click.echo(f"\nListing models: {url}\n")
click.echo(f"\nRetrieving model: {url}\n")
click.echo(f"Headers: {headers}")
Copy link
Member

Choose a reason for hiding this comment

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

remove

try:
response = requests.get(url, headers=headers)
if session_id:
if response.status_code == 200:
json_data = response.json()
count, result = json_data.values()
click.echo(f"Found {count} models")
click.echo("\n---------------------------------\n")
for obj in result:
if obj.get("session_id")==session_id:
click.echo("{")
for k, v in obj.items():
click.echo(f"\t{k}: {v}")
click.echo("}")

elif response.status_code == 500:
json_data = response.json()
click.echo(f'Error: {json_data["message"]}')
else:
click.echo(f"Error: {response.status_code}")
elif id:
print_response(response, "model", True, session_id)
else:
print_response(response, "models", False, session_id)
print_response(response, "model", id)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")
42 changes: 35 additions & 7 deletions fedn/cli/package_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ def create_cmd(ctx, path, name):
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Package ID")
@click.option("--n_max", required=False, help="Number of items to list")
@package_cmd.command("list")
@click.pass_context
def list_packages(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None, n_max: int = None):
def list_packages(ctx, protocol: str, host: str, port: str, token: str = None, n_max: int = None):
"""Return:
------
- count: number of packages
Expand All @@ -62,6 +61,38 @@ def list_packages(ctx, protocol: str, host: str, port: str, token: str = None, i
if n_max:
headers["X-Limit"] = n_max

_token = get_token(token)

if _token:
headers["Authorization"] = _token


click.echo(f"\nListing packages: {url}\n")
click.echo(f"Headers: {headers}")
try:
response = requests.get(url, headers=headers)
print_response(response, "packages", None)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")


@click.option("-p", "--protocol", required=False, default=CONTROLLER_DEFAULTS["protocol"], help="Communication protocol of controller (api)")
@click.option("-H", "--host", required=False, default=CONTROLLER_DEFAULTS["host"], help="Hostname of controller (api)")
@click.option("-P", "--port", required=False, default=CONTROLLER_DEFAULTS["port"], help="Port of controller (api)")
@click.option("-t", "--token", required=False, help="Authentication token")
@click.option("-id", "--id", required=False, help="Package ID")
@package_cmd.command("get")
@click.pass_context
def get_package(ctx, protocol: str, host: str, port: str, token: str = None, id: str = None):
"""Return:
------
- result: package with given id

"""
url = get_api_url(protocol=protocol, host=host, port=port, endpoint="packages")
headers = {}


_token = get_token(token)

if _token:
Expand All @@ -72,13 +103,10 @@ def list_packages(ctx, protocol: str, host: str, port: str, token: str = None, i
headers["id"] = id


click.echo(f"\nListing packages: {url}\n")
click.echo(f"\nretrieving package: {url}\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

retrieving => Retrieving? :)

click.echo(f"Headers: {headers}")
try:
response = requests.get(url, headers=headers)
if id:
print_response(response, "package", True)
else:
print_response(response, "packages", False)
print_response(response, "package", id)
except requests.exceptions.ConnectionError:
click.echo(f"Error: Could not connect to {url}")
Loading
Loading