refactor(server): replace print statements with logger (#1250)

# What does this PR do?

- Introduced logging in `StackRun` to replace print-based messages
- Improved error handling for config file loading and parsing
- Replaced `cprint` with `logger.error` for consistent error messaging
- Ensured logging is used in `server.py` for startup, shutdown, and
runtime messages
- Added missing exception handling for invalid providers

Signed-off-by: Sébastien Han <seb@redhat.com>

Signed-off-by: Sébastien Han <seb@redhat.com>
This commit is contained in:
Sébastien Han 2025-02-26 06:31:37 +01:00 committed by GitHub
parent eb743a3b26
commit 929c5f0842
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 35 additions and 33 deletions

View file

@ -5,6 +5,7 @@
# the root directory of this source tree. # the root directory of this source tree.
import argparse import argparse
import logging
import os import os
from pathlib import Path from pathlib import Path
@ -12,6 +13,8 @@ from llama_stack.cli.subcommand import Subcommand
REPO_ROOT = Path(__file__).parent.parent.parent.parent REPO_ROOT = Path(__file__).parent.parent.parent.parent
logger = logging.getLogger(__name__)
class StackRun(Subcommand): class StackRun(Subcommand):
def __init__(self, subparsers: argparse._SubParsersAction): def __init__(self, subparsers: argparse._SubParsersAction):
@ -70,12 +73,10 @@ class StackRun(Subcommand):
type=str, type=str,
help="Image Type used during the build. This can be either conda or container or venv.", help="Image Type used during the build. This can be either conda or container or venv.",
choices=["conda", "container", "venv"], choices=["conda", "container", "venv"],
default="conda",
) )
def _run_stack_run_cmd(self, args: argparse.Namespace) -> None: def _run_stack_run_cmd(self, args: argparse.Namespace) -> None:
import yaml import yaml
from termcolor import cprint
from llama_stack.distribution.build import ImageType from llama_stack.distribution.build import ImageType
from llama_stack.distribution.configure import parse_and_maybe_upgrade_config from llama_stack.distribution.configure import parse_and_maybe_upgrade_config
@ -85,10 +86,6 @@ class StackRun(Subcommand):
) )
from llama_stack.distribution.utils.exec import formulate_run_args, run_with_pty from llama_stack.distribution.utils.exec import formulate_run_args, run_with_pty
if not args.config:
self.parser.error("Must specify a config file to run")
return
config_file = Path(args.config) config_file = Path(args.config)
has_yaml_suffix = args.config.endswith(".yaml") has_yaml_suffix = args.config.endswith(".yaml")
template_name = None template_name = None
@ -115,11 +112,23 @@ class StackRun(Subcommand):
self.parser.error( self.parser.error(
f"File {str(config_file)} does not exist.\n\nPlease run `llama stack build` to generate (and optionally edit) a run.yaml file" f"File {str(config_file)} does not exist.\n\nPlease run `llama stack build` to generate (and optionally edit) a run.yaml file"
) )
return
print(f"Using run configuration: {config_file}") if not config_file.is_file():
self.parser.error(
f"Config file must be a valid file path, '{config_file} is not a file: type={type(config_file)}"
)
logger.info(f"Using run configuration: {config_file}")
try:
config_dict = yaml.safe_load(config_file.read_text()) config_dict = yaml.safe_load(config_file.read_text())
except yaml.parser.ParserError as e:
self.parser.error(f"failed to load config file '{config_file}':\n {e}")
try:
config = parse_and_maybe_upgrade_config(config_dict) config = parse_and_maybe_upgrade_config(config_dict)
except AttributeError as e:
self.parser.error(f"failed to parse config file '{config_file}':\n {e}")
run_args = formulate_run_args(args.image_type, args.image_name, config, template_name) run_args = formulate_run_args(args.image_type, args.image_name, config, template_name)
@ -129,18 +138,10 @@ class StackRun(Subcommand):
for env_var in args.env: for env_var in args.env:
if "=" not in env_var: if "=" not in env_var:
cprint( self.parser.error(f"Environment variable '{env_var}' must be in KEY=VALUE format")
f"Environment variable '{env_var}' must be in KEY=VALUE format",
color="red",
)
return
key, value = env_var.split("=", 1) # split on first = only key, value = env_var.split("=", 1) # split on first = only
if not key: if not key:
cprint( self.parser.error(f"Environment variable '{env_var}' has empty key")
f"Environment variable '{env_var}' has empty key",
color="red",
)
return
run_args.extend(["--env", f"{key}={value}"]) run_args.extend(["--env", f"{key}={value}"])
if args.tls_keyfile and args.tls_certfile: if args.tls_keyfile and args.tls_certfile:

View file

@ -142,7 +142,7 @@ def handle_signal(app, signum, _) -> None:
not block the current execution. not block the current execution.
""" """
signame = signal.Signals(signum).name signame = signal.Signals(signum).name
print(f"Received signal {signame} ({signum}). Exiting gracefully...") logger.info(f"Received signal {signame} ({signum}). Exiting gracefully...")
async def shutdown(): async def shutdown():
try: try:
@ -184,9 +184,9 @@ def handle_signal(app, signum, _) -> None:
@asynccontextmanager @asynccontextmanager
async def lifespan(app: FastAPI): async def lifespan(app: FastAPI):
print("Starting up") logger.info("Starting up")
yield yield
print("Shutting down") logger.info("Shutting down")
for impl in app.__llama_stack_impls__.values(): for impl in app.__llama_stack_impls__.values():
await impl.shutdown() await impl.shutdown()
@ -352,10 +352,10 @@ def main():
for env_pair in args.env: for env_pair in args.env:
try: try:
key, value = validate_env_pair(env_pair) key, value = validate_env_pair(env_pair)
print(f"Setting CLI environment variable {key} => {value}") logger.info(f"Setting CLI environment variable {key} => {value}")
os.environ[key] = value os.environ[key] = value
except ValueError as e: except ValueError as e:
print(f"Error: {str(e)}") logger.error(f"Error: {str(e)}")
sys.exit(1) sys.exit(1)
if args.yaml_config: if args.yaml_config:
@ -363,12 +363,12 @@ def main():
config_file = Path(args.yaml_config) config_file = Path(args.yaml_config)
if not config_file.exists(): if not config_file.exists():
raise ValueError(f"Config file {config_file} does not exist") raise ValueError(f"Config file {config_file} does not exist")
print(f"Using config file: {config_file}") logger.info(f"Using config file: {config_file}")
elif args.template: elif args.template:
config_file = Path(REPO_ROOT) / "llama_stack" / "templates" / args.template / "run.yaml" config_file = Path(REPO_ROOT) / "llama_stack" / "templates" / args.template / "run.yaml"
if not config_file.exists(): if not config_file.exists():
raise ValueError(f"Template {args.template} does not exist") raise ValueError(f"Template {args.template} does not exist")
print(f"Using template {args.template} config file: {config_file}") logger.info(f"Using template {args.template} config file: {config_file}")
else: else:
raise ValueError("Either --yaml-config or --template must be provided") raise ValueError("Either --yaml-config or --template must be provided")
@ -376,9 +376,9 @@ def main():
config = replace_env_vars(yaml.safe_load(fp)) config = replace_env_vars(yaml.safe_load(fp))
config = StackRunConfig(**config) config = StackRunConfig(**config)
print("Run configuration:") logger.info("Run configuration:")
safe_config = redact_sensitive_fields(config.model_dump()) safe_config = redact_sensitive_fields(config.model_dump())
print(yaml.dump(safe_config, indent=2)) logger.info(yaml.dump(safe_config, indent=2))
app = FastAPI(lifespan=lifespan) app = FastAPI(lifespan=lifespan)
app.add_middleware(TracingMiddleware) app.add_middleware(TracingMiddleware)
@ -387,7 +387,8 @@ def main():
try: try:
impls = asyncio.run(construct_stack(config)) impls = asyncio.run(construct_stack(config))
except InvalidProviderError: except InvalidProviderError as e:
logger.error(f"Error: {str(e)}")
sys.exit(1) sys.exit(1)
if Api.telemetry in impls: if Api.telemetry in impls:
@ -432,7 +433,7 @@ def main():
) )
) )
cprint(f"Serving API {api_str}", "white", attrs=["bold"]) logger.info(f"Serving API {api_str}")
for endpoint in endpoints: for endpoint in endpoints:
cprint(f" {endpoint.method.upper()} {endpoint.route}", "white") cprint(f" {endpoint.method.upper()} {endpoint.route}", "white")
@ -462,10 +463,10 @@ def main():
"ssl_keyfile": keyfile, "ssl_keyfile": keyfile,
"ssl_certfile": certfile, "ssl_certfile": certfile,
} }
print(f"HTTPS enabled with certificates:\n Key: {keyfile}\n Cert: {certfile}") logger.info(f"HTTPS enabled with certificates:\n Key: {keyfile}\n Cert: {certfile}")
listen_host = ["::", "0.0.0.0"] if not args.disable_ipv6 else "0.0.0.0" listen_host = ["::", "0.0.0.0"] if not args.disable_ipv6 else "0.0.0.0"
print(f"Listening on {listen_host}:{port}") logger.info(f"Listening on {listen_host}:{port}")
uvicorn_config = { uvicorn_config = {
"app": app, "app": app,