Closed flapili closed 1 year ago
I did this fix :
@orion_app.command()
@server_app.command()
async def start(
host: str = SettingsOption(PREFECT_SERVER_API_HOST),
port: int = SettingsOption(PREFECT_SERVER_API_PORT),
keep_alive_timeout: int = SettingsOption(PREFECT_SERVER_API_KEEPALIVE_TIMEOUT),
log_level: str = SettingsOption(PREFECT_LOGGING_SERVER_LEVEL),
scheduler: bool = SettingsOption(PREFECT_API_SERVICES_SCHEDULER_ENABLED),
analytics: bool = SettingsOption(
PREFECT_SERVER_ANALYTICS_ENABLED, "--analytics-on/--analytics-off"
),
late_runs: bool = SettingsOption(PREFECT_API_SERVICES_LATE_RUNS_ENABLED),
ui: bool = SettingsOption(PREFECT_UI_ENABLED),
):
"""Start a Prefect server"""
server_env = os.environ.copy()
server_env["PREFECT_API_SERVICES_SCHEDULER_ENABLED"] = str(scheduler)
server_env["PREFECT_SERVER_ANALYTICS_ENABLED"] = str(analytics)
server_env["PREFECT_API_SERVICES_LATE_RUNS_ENABLED"] = str(late_runs)
server_env["PREFECT_API_SERVICES_UI"] = str(ui)
server_env["PREFECT_LOGGING_SERVER_LEVEL"] = log_level
base_url = f"http://{host}:{port}"
# Check if uvicorn is installed in a venv
if os.name == "nt":
uvicorn_path = Path(sys.executable).parent / "Scripts" / "uvicorn.exe"
else:
uvicorn_path = Path(sys.executable).parent / "uvicorn"
if not uvicorn_path.exists(): # should try to run uvicorn --version and catch the error instead ?
uvicorn_path = "uvicorn" # fallback to global uvicorn installation
async with anyio.create_task_group() as tg:
app.console.print(generate_welcome_blurb(base_url, ui_enabled=ui))
app.console.print("\n")
server_process_id = await tg.start(
partial(
run_process,
command=[
str(uvicorn_path),
"--app-dir",
# quote wrapping needed for windows paths with spaces
f'"{prefect.__module_path__.parent}"',
"--factory",
"prefect.server.api.server:create_app",
"--host",
str(host),
"--port",
str(port),
"--timeout-keep-alive",
str(keep_alive_timeout),
],
env=server_env,
stream_output=True,
)
)
# Explicitly handle the interrupt signal here, as it will allow us to
# cleanly stop the uvicorn server. Failing to do that may cause a
# large amount of anyio error traces on the terminal, because the
# SIGINT is handled by Typer/Click in this process (the parent process)
# and will start shutting down subprocesses:
# https://github.com/PrefectHQ/server/issues/2475
setup_signal_handlers_server(
server_process_id, "the Prefect server", app.console.print
)
app.console.print("Server stopped!")
any advise about the implementation / idea before I submit an PR ?
@flapili you can probably achieve something similar with
import sys
# ...existing code
server_process_id = await tg.start(
partial(
run_process,
command=[
sys.executable,
"-m",
"uvicorn",
"--app-dir",
# quote wrapping needed for windows paths with spaces
f'"{prefect.__module_path__.parent}"',
"--factory",
"prefect.server.api.server:create_app",
"--host",
str(host),
"--port",
str(port),
"--timeout-keep-alive",
str(keep_alive_timeout),
],
env=server_env,
stream_output=True,
)
)
This should run Uvicorn using the same Python interpreter you ran the Prefect CLI with, and doesn't need an OS-dependent lookup for the uvicorn executable. I just tested this by editing the server CLI code and it worked as expected on both Windows and Linux.
I recommend waiting for input for someone from the Prefect team before making a PR; there might be a better way to do this that I'm overlooking.
I totally forgot about-m
, you rock ! 👍
@flapili FWIW I believe that both your solution and my suggestion have the same issue with spaces in Windows paths that you see later in the command array. And I don't think the string-inside-a-string approach used for the module path arg will work universally for the command itself. It will work on Windows, but not elsewhere.
So you'd probably still need something like:
if os.name == "nt":
python_path = f'"{sys.executable}"'
else:
python_path = sys.executable
server_process_id = await tg.start(
partial(
run_process,
command=[
python_path,
# ...other code
or even just
server_process_id = await tg.start(
partial(
run_process,
command=[
f'"{sys.executable}"' if os.name == "nt" else sys.executable,
And that should work no matter where you run it.
if os.name == "nt":
python_path = f'"{sys.executable}"'
else:
python_path = sys.executable
eek. We use sys.executable
for spawning flow run processes too. We may want to have a special run_python_process
utility that handles this?
@madkinsz Yeah, seems like a maybe good opportunity to extract this into a reusable function in prefect.utilities.processutils
?
This issue affects the
if os.name == "nt": python_path = f'"{sys.executable}"' else: python_path = sys.executable
eek. We use
sys.executable
for spawning flow run processes too. We may want to have a specialrun_python_process
utility that handles this?
Yes, there is an issue there as well
First check
Bug summary
if we don't have uvicorn available (either installed globally or via venv activation) the command
prefect server start
crashReproduction
Error
Versions
Additional context
according to the doc :
sometime we can't use activated venv (in systemd service as example) or it's not practical and using the full path is more simple.