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-944 + SK-934 | Add flag for deleting the virtual env #661

Merged
merged 19 commits into from
Sep 2, 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
53 changes: 20 additions & 33 deletions fedn/cli/run_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from fedn.cli.main import main
from fedn.cli.shared import apply_config


def get_statestore_config_from_file(init):
""":param init:
:return:
Expand Down Expand Up @@ -44,6 +43,14 @@ def check_yaml_exists(path):
click.echo(f"Could not find fedn.yaml in {path}")
exit(-1)
return yaml_file
def delete_virtual_environment(remove_venv,dispatcher):
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.")
@main.group("run")
@click.pass_context
def run_cmd(ctx):
Expand All @@ -54,7 +61,7 @@ def run_cmd(ctx):
@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("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
@click.option("-v", "--remove-venv", is_flag=True, default=True,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 don't think you need default=True when is_flag=True

@click.pass_context
def validate_cmd(ctx, path,input,output,remove_venv):
"""Execute 'validate' entrypoint in fedn.yaml.
Expand All @@ -75,18 +82,12 @@ def validate_cmd(ctx, path,input,output,remove_venv):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("validate {} {}".format(input, output))
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.")
delete_virtual_environment(remove_venv,dispatcher)
@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("-v", "--remove-venv", default=True, type=bool, required=False, help="flag if set to False doesn't remove venv")
@click.option("-v", "--remove-venv", is_flag=True, default=True,required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def train_cmd(ctx, path,input,output,remove_venv):
"""Execute 'train' entrypoint in fedn.yaml.
Expand All @@ -107,16 +108,11 @@ def train_cmd(ctx, path,input,output,remove_venv):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("train {} {}".format(input, output))
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.")
delete_virtual_environment(remove_venv,dispatcher)

@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.option("-v", "--remove-venv", is_flag=True, default=True,required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def startup_cmd(ctx, path,remove_venv):
"""Execute 'startup' entrypoint in fedn.yaml.
Expand All @@ -137,17 +133,12 @@ def startup_cmd(ctx, path,remove_venv):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("startup")
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.")
delete_virtual_environment(remove_venv,dispatcher)


@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.option("-v", "--remove-venv", is_flag=True, default=True,required=False, help="flag if set to False doesn't remove venv")
@click.pass_context
def build_cmd(ctx, path,remove_venv):
"""Execute 'build' entrypoint in fedn.yaml.
Expand All @@ -168,13 +159,9 @@ def build_cmd(ctx, path,remove_venv):
dispatcher = Dispatcher(config, path)
_ = dispatcher._get_or_create_python_env()
dispatcher.run_cmd("build")
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.")
print(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.

remove print

delete_virtual_environment(remove_venv,dispatcher)



@run_cmd.command("client")
Expand Down
9 changes: 3 additions & 6 deletions fedn/network/api/gunicorn_app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from gunicorn.app.base import BaseApplication
import os
class GunicornApp(BaseApplication):
def __init__(self, app, options=None):
self.options = options or {}
Expand All @@ -15,12 +14,10 @@ def load_config(self):
def load(self):
return self.application

def run_gunicorn(app, workers=4):
workers=os.cpu_count()
def run_gunicorn(app, host,port,workers=4):
bind_address=str(host)+":"+str(port)
Copy link
Member

Choose a reason for hiding this comment

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

this is ok but a better way is:

bind_address = f"{host}:{port}"

options = {
"bind": "127.0.0.1:8000", # Specify the bind address and port here
"bind": bind_address, # Specify the bind address and port here
"workers": workers,
}
GunicornApp(app, options).run()
if __name__ == "main":
run_gunicorn()
3 changes: 2 additions & 1 deletion fedn/network/api/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ def start_server_api():
if debug:
app.run(debug=debug, port=port, host=host)
else:
gunicorn_app.run_gunicorn(app)
workers=os.cpu_count()
gunicorn_app.run_gunicorn(app,host,port,workers)
Wrede marked this conversation as resolved.
Show resolved Hide resolved
if __name__ == "__main__":
start_server_api()
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ requires-python = '>=3.8,<3.13'
dependencies = [
"requests",
"urllib3>=1.26.4",
"gunicorn>=20.0.4,<21.0.0",
"gunicorn>=20.0.4",
"minio",
"grpcio~=1.60.0",
"grpcio-tools~=1.60.0",
Expand Down
Loading