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

Feature/SK-944 + SK-934 | Add flag for deleting the virtual env #661

Merged
merged 19 commits into from
Sep 2, 2024
Merged

Feature/SK-944 + SK-934 | Add flag for deleting the virtual env #661

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
b963f4b
flag for not deleting the venv
sowmyasris Jul 17, 2024
395c755
Merge branch 'master' of https://github.com/scaleoutsystems/fedn into…
sowmyasris Jul 17, 2024
060907d
added flag to reuse the existing env without deleting , default is to…
sowmyasris Jul 17, 2024
55e179a
fixed the build command and added the path flag
sowmyasris Jul 19, 2024
da5654d
changed flag to remove-venv
sowmyasris Jul 19, 2024
a67f1df
wrapped check yaml exist to separate method
sowmyasris Jul 26, 2024
5b21200
added Gunicorn server as production server, if the debug is False in …
sowmyasris Jul 31, 2024
9961836
ruff double quotes error corrected
sowmyasris Jul 31, 2024
69795a1
fixed the function exxecution during import
sowmyasris Jul 31, 2024
488d9f2
added gunicorn dependency in pyproject.toml
sowmyasris Jul 31, 2024
e8eb846
fixed the import error
sowmyasris Jul 31, 2024
41d0db7
updated the code with the changes suggested
sowmyasris Aug 12, 2024
aa947c7
removed unused package
sowmyasris Aug 12, 2024
1d353ae
changed the virtual-env flag to keep-venv and some code clean up
sowmyasris Aug 14, 2024
37c9bd1
run_cmd updated with keep-venv flag
sowmyasris Aug 18, 2024
e67db30
code clean up
sowmyasris Aug 19, 2024
c685cff
Merge branch 'master' of https://github.com/scaleoutsystems/fedn into…
sowmyasris Aug 19, 2024
317904c
updated help command
sowmyasris Aug 19, 2024
7847b70
fixed indent issue
sowmyasris Sep 2, 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
4 changes: 2 additions & 2 deletions fedn/cli/client_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
from fedn.common.exceptions import InvalidClientConfig
from fedn.network.clients.client import Client

from .main import main
from .shared import CONTROLLER_DEFAULTS, apply_config, get_api_url, get_token, print_response
from fedn.cli.main import main
from fedn.cli.shared import CONTROLLER_DEFAULTS, apply_config, get_api_url, get_token, print_response


def validate_client_config(config):
Expand Down
2 changes: 1 addition & 1 deletion fedn/cli/controller_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import click

from .main import main
from fedn.cli.main import main


@main.group("controller")
Expand Down
99 changes: 53 additions & 46 deletions fedn/cli/run_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from fedn.network.combiner.combiner import Combiner
from fedn.utils.dispatcher import Dispatcher, _read_yaml_file

from .client_cmd import validate_client_config
from .main import main
from .shared import apply_config
from fedn.cli.client_cmd import validate_client_config
from fedn.cli.main import main
from fedn.cli.shared import apply_config


def get_statestore_config_from_file(init):
Expand All @@ -36,7 +36,14 @@ def check_helper_config_file(config):
exit(-1)
return helper


def check_yaml_exists(path):
"""Check if fedn.yaml exists in the given path."""
yaml_file = os.path.join(path, "fedn.yaml")
if not os.path.exists(yaml_file):
logger.error(f"Could not find fedn.yaml in {path}")
click.echo(f"Could not find fedn.yaml in {path}")
exit(-1)
return yaml_file
@main.group("run")
@click.pass_context
def run_cmd(ctx):
Expand All @@ -46,20 +53,18 @@ def run_cmd(ctx):
@run_cmd.command("validate")
@click.option("-p", "--path", required=True, help="Path to package directory containing fedn.yaml")
@click.option("-i", "--input", required=True, help="Path to input model" )
@click.option("-o", "--output", required=True,help="Path to write the output JSON containing validation metrics")
@click.option("-o", "--output", required=True, help="Path to write the output JSON containing validation metrics")
@click.option("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
Copy link
Member

Choose a reason for hiding this comment

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

I would use is_flag=True here, since it's bool. It will be a better UX

@click.pass_context
def validate_cmd(ctx, path,input,output):
def validate_cmd(ctx, path,input,output,remove_venv):
"""Execute 'validate' entrypoint in fedn.yaml.

:param ctx:
:param path: Path to folder containing fedn.yaml
:type path: str
"""
path = os.path.abspath(path)
yaml_file = os.path.join(path, "fedn.yaml")
if not os.path.exists(yaml_file):
logger.error(f"Could not find fedn.yaml in {path}")
exit(-1)
yaml_file = check_yaml_exists(path)

config = _read_yaml_file(yaml_file)
# Check that validate is defined in fedn.yaml under entry_points
Expand All @@ -70,28 +75,28 @@ def validate_cmd(ctx, path,input,output):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("validate {} {}".format(input, output))

# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
if remove_venv:
Copy link
Member

Choose a reason for hiding this comment

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

perhaps move this if statement to a separate function that can be used within each subcommand? A lot of redundancy now

# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
else:
logger.warning("No virtualenv found to remove.")
@run_cmd.command("train")
@click.option("-p", "--path", required=True, help="Path to package directory containing fedn.yaml")
@click.option("-i", "--input", required=True, help="Path to input model parameters" )
@click.option("-o", "--output", required=True,help="Path to write the updated model parameters ")
@click.option("-o", "--output", required=True, help="Path to write the updated model parameters ")
@click.option("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def train_cmd(ctx, path,input,output):
def train_cmd(ctx, path,input,output,remove_venv):
"""Execute 'train' entrypoint in fedn.yaml.

:param ctx:
:param path: Path to folder containing fedn.yaml
:type path: str
"""
path = os.path.abspath(path)
yaml_file = os.path.join(path, "fedn.yaml")
if not os.path.exists(yaml_file):
logger.error(f"Could not find fedn.yaml in {path}")
exit(-1)
yaml_file = check_yaml_exists(path)

config = _read_yaml_file(yaml_file)
# Check that train is defined in fedn.yaml under entry_points
Expand All @@ -102,26 +107,26 @@ def train_cmd(ctx, path,input,output):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("train {} {}".format(input, output))

# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
if remove_venv:
# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
else:
logger.warning("No virtualenv found to remove.")
@run_cmd.command("startup")
@click.option("-p", "--path", required=True, help="Path to package directory containing fedn.yaml")
@click.option("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def startup_cmd(ctx, path):
def startup_cmd(ctx, path,remove_venv):
"""Execute 'startup' entrypoint in fedn.yaml.

:param ctx:
:param path: Path to folder containing fedn.yaml
:type path: str
"""
path = os.path.abspath(path)
yaml_file = os.path.join(path, "fedn.yaml")
if not os.path.exists(yaml_file):
logger.error(f"Could not find fedn.yaml in {path}")
exit(-1)
yaml_file = check_yaml_exists(path)

config = _read_yaml_file(yaml_file)
# Check that startup is defined in fedn.yaml under entry_points
Expand All @@ -132,27 +137,27 @@ def startup_cmd(ctx, path):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("startup")

# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
if remove_venv:
# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
else:
logger.warning("No virtualenv found to remove.")

@run_cmd.command("build")
@click.option("-p", "--path", required=True, help="Path to package directory containing fedn.yaml")
@click.option("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def build_cmd(ctx, path):
def build_cmd(ctx, path,remove_venv):
"""Execute 'build' entrypoint in fedn.yaml.

:param ctx:
:param path: Path to folder containing fedn.yaml
:type path: str
"""
path = os.path.abspath(path)
yaml_file = os.path.join(path, "fedn.yaml")
if not os.path.exists(yaml_file):
logger.error(f"Could not find fedn.yaml in {path}")
exit(-1)
yaml_file = check_yaml_exists(path)

config = _read_yaml_file(yaml_file)
# Check that build is defined in fedn.yaml under entry_points
Expand All @@ -163,11 +168,13 @@ def build_cmd(ctx, path):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("build")

# delete the virtualenv
if dispatcher.python_env_path:
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
if remove_venv:
# delete the virtualenv
if dispatcher.python_env_path:
Wrede marked this conversation as resolved.
Show resolved Hide resolved
logger.info(f"Removing virtualenv {dispatcher.python_env_path}")
shutil.rmtree(dispatcher.python_env_path)
else:
logger.warning("No virtualenv found to remove.")


@run_cmd.command("client")
Expand Down
Loading
Loading