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

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

Closed
4 tasks done
rpeden opened this issue Sep 14, 2022 · 5 comments · Fixed by #7224
Closed
4 tasks done
Assignees
Labels
arch:windows Related to the Windows OS bug Something isn't working good first issue This issue is good for newcomers status:accepted We may work on this; we will accept work from external contributors

Comments

@rpeden
Copy link
Contributor

rpeden commented Sep 14, 2022

First check

  • I added a descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the Prefect documentation for this issue.
  • 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:

str(prefect.__module_path__.parent),

@rpeden rpeden added bug Something isn't working status:triage labels Sep 14, 2022
@rpeden rpeden changed the title Prefect Orion fails to start on Windows due to space in Prefect install path Prefect Orion fails to start on Windows when there is a space in Prefect package's path Sep 14, 2022
@rpeden rpeden added the v2 label Sep 14, 2022
@bunchesofdonald bunchesofdonald added arch:windows Related to the Windows OS status:accepted We may work on this; we will accept work from external contributors and removed status:triage labels Sep 14, 2022
@bunchesofdonald
Copy link
Contributor

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

@zanieb zanieb added the good first issue This issue is good for newcomers label Sep 14, 2022
@bansalkanav
Copy link

Kindly allow me to fix this bug.

@zanieb
Copy link
Contributor

zanieb commented Sep 18, 2022

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

bansalkanav added a commit to bansalkanav/prefect that referenced this issue Sep 19, 2022
Prefect Orion fails to start on Windows when there is a space in Prefect package's path.

PrefectHQ#6821
bansalkanav added a commit to bansalkanav/prefect that referenced this issue Sep 19, 2022
@bansalkanav
Copy link

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

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

@hallenmaia
Copy link
Contributor

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.

# Passing a string to open_process is equivalent to shell=True which is
# generally necessary for Unix-like commands on Windows but otherwise should
# be avoided
if sys.platform == "win32":
command = " ".join(command)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch:windows Related to the Windows OS bug Something isn't working good first issue This issue is good for newcomers status:accepted We may work on this; we will accept work from external contributors
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants