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

Fix Issue 1847 - Adding Current Path when Invoking Flask-Cli Run #1863

Closed

Conversation

dorianpula
Copy link

Fix for: #1847

Adds the current directory to the Python path, when invoking a local development server. flask run was unable to find the app's module in the PYTHONPATH, while python -m flask run adds the current working directory to the PYTHONPATH automatically.

@ThiefMaster
Copy link
Member

Strong 👎; IMO people should add a setup.py

@@ -398,6 +398,8 @@ def run_command(info, host, port, reload, debugger, eager_loading,
The reloader and debugger are by default enabled if the debug flag of
Flask is enabled and disabled otherwise.
"""
import sys

This comment was marked as off-topic.

@dorianpula
Copy link
Author

dorianpula commented Jun 2, 2016

Sure, but the fact is that the behaviour between python -m flask run and flask run is inconsistent. Not an ideal fix, but then again not every flask app should need a setup.py, if it not being distributed via PyPI.

@ThiefMaster
Copy link
Member

A setup.py makes perfect sense even if you never intend to publish your package on PyPI. It's how your register a package properly within your Python environment.

@untitaker
Copy link
Contributor

You can already set FLASK_APP=file.py (or FLASK_APP=package/__init__.py), and the appropriate path will be added to sys.path.

setup.py is very useful in a lot of cases, but it is not required.

All of this is poorly documented and a few instances in the quickstart are completely wrong (i.e. nonfunctional) in 0.11 and only fixed in master. If you find any other instances where this behavior could be clarified, feel free to point them out. Thanks!

@untitaker untitaker closed this Jun 3, 2016
@davidism
Copy link
Member

davidism commented Jun 3, 2016

@untitaker there appears to be some path issues with pointing at __init__.py. #1847 (comment) Pointing at __init__ seems unintuitive, given that you would normally never do that in imports or -m calls.

@davidism
Copy link
Member

davidism commented Jun 3, 2016

I would expect to be able to point to FLASK_APP=package, and I'm sure the vast majority of "didn't use setup.py" cases are running from the app directory. Given that FLASK_APP=package python -m flask run works in this case, and is doing the same thing to the path, this seems like an ok workaround to make both behave the same.

Maybe we could be smarter about when we add the current directory to the path, or output a warning to use setup.py instead when we do.

@davidism
Copy link
Member

davidism commented Jun 3, 2016

Either way, we need a clearer message when this fails, rather than just dumping an ImportError traceback.

Could not locate a Flask app at [location]. Install your app in development mode, or see [the docs] for more information.

@jeffwidman
Copy link
Contributor

Not a fan of forcing folks to use a setup.py, I prefer that we let folks get up and running with a simple flask run within their app directory.

@untitaker
Copy link
Contributor

See #1872, I think this got fixed.

@untitaker
Copy link
Contributor

untitaker commented Jun 3, 2016

@jeffwidman I don't think that's possible at all. How would the command know which package/file to import?

@jeffwidman
Copy link
Contributor

Yeah, you're right. I am so used to accessing run via a simple custom manage.py run command that I wrote that I didn't think through how that might work if someone doesn't write their own custom command.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants