prefect icon indicating copy to clipboard operation
prefect copied to clipboard

Prefect Orion fails to start on Windows when there is a space in Prefect package's path

Open rpeden opened this issue 2 years ago • 4 comments

First check

  • [X] I added a descriptive title to this issue.
  • [X] I used the GitHub search to find a similar issue and didn't find it.
  • [X] I searched the Prefect documentation for this issue.
  • [X] I checked that this issue is related to Prefect and not one of its dependencies.

Bug summary

If the Prefect package is installed on Windows in a directory containing a space in the pathname, Prefect Orion fails to start because it cannot launch Uvicorn.

The problem happens when we set the --app-dir flag just before launching Uvicorn.

Reproduction

  • Run cmd or Powershell on Windows
  • Create a virtual environment in a directory containing a space character in the path.
  • Activate the virtual environment.
  • Install Prefect.
  • Run prefect orion start

You can also observe the problem directly by running something like:

uvicorn --app-dir c:\my python site packages\prefect --factory prefect.orion.api.server:create_app

Error

___ ___ ___ ___ ___ ___ _____    ___  ___ ___ ___  _  _
| _ \ _ \ __| __| __/ __|_   _|  / _ \| _ \_ _/ _ \| \| |
|  _/   / _|| _|| _| (__  | |   | (_) |   /| | (_) | .` |
|_| |_|_\___|_| |___\___| |_|    \___/|_|_\___\___/|_|\_|
Configure Prefect to communicate with the server with:
    prefect config set PREFECT_API_URL=http://127.0.0.1:4200/api
View the API reference documentation at http://127.0.0.1:4200/docs
Check out the dashboard at http://127.0.0.1:4200/
Usage: uvicorn [OPTIONS] APP
Try 'uvicorn --help' for help.
Error: Got unexpected extra argument (prefect.orion.api.server:create_app)
Orion stopped!

Versions

Version:             2.4.0
API version:         0.8.0
Python version:      3.10.4
Git commit:          513639e8
Built:               Tue, Sep 13, 2022 2:15 PM
OS/Arch:             win32/AMD64
Profile:             default
Server type:         ephemeral
Server:
  Database:          sqlite
  SQLite version:    3.39.2

Additional context

Place where this occurs in Prefect's code: https://github.com/PrefectHQ/prefect/blob/2f27976a4dd376fea4fd06fd265dfe8a2751a764/src/prefect/cli/orion.py#L127

rpeden avatar Sep 14 '22 03:09 rpeden

Yep, that's definitely a bug, thanks for reporting it Ryan!

bunchesofdonald avatar Sep 14 '22 13:09 bunchesofdonald

Kindly allow me to fix this bug.

bansalkanav avatar Sep 18 '22 19:09 bansalkanav

Go for it! Let us know if you run into issues.

zanieb avatar Sep 18 '22 20:09 zanieb

@madkinsz Let me know if there is something else you are expecting.

Fixed this bug by updating the absolute path to the relative path.

bansalkanav avatar Sep 19 '22 08:09 bansalkanav

The code below converts the command to a string on the Windows platform and this is affecting how the command is interpreted in the shell.

https://github.com/PrefectHQ/prefect/blob/9b5123c0f79f61f6138306764c0d0a62fd09727a/src/prefect/utilities/processutils.py#L24-L28

The comment in the code says that "shell=True is generally required for Unix-like commands on Windows" but the submodule.Popen documentation treats it differently:

On POSIX with shell=True, the shell defaults to /bin/sh. If args is a string, the string specifies the command to execute through the shell. This means that the string must be formatted exactly as it would be when typed at the shell prompt. This includes, for example, quoting or backslash escaping filenames with spaces in them. If args is a sequence, the first item specifies the command string, and any additional items will be treated as additional arguments to the shell itself.

On Windows with shell=True, the COMSPEC environment variable specifies the default shell. The only time you need to specify shell=True on Windows is when the command you wish to execute is built into the shell (e.g. dir or copy). You do not need shell=True to run a batch file or console-based executable.

When converting the command to string, the function does not quote the path correctly:

uvicorn --app-dir c:\users\halle\pycharm projects\prefect\src --factory prefect.orion.api.server:create_app --host 127.0.0.1 --port 4200

Without this conversion (I removed it from the code) the command runs correctly, both on POSIX and Windows systems:

uvicorn --app-dir 'c:\users\halle\pycharm projects\prefect\src' --factory prefect.orion.api.server:create_app --host 127.0.0.1 --port 4200

I think to solve this problem, the conversion should not take place in that part of the code. This responsibility must be on the one who calls this function to pass the command correctly.

hallenmaia avatar Oct 05 '22 18:10 hallenmaia