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

eager loading is not documented #3430

Closed
gward opened this issue Nov 18, 2019 · 6 comments · Fixed by #3434
Closed

eager loading is not documented #3430

gward opened this issue Nov 18, 2019 · 6 comments · Fixed by #3434
Labels

Comments

@gward
Copy link

gward commented Nov 18, 2019

I have been annoyed for a while by Flask's lazy loading feature, without even knowing about the feature or why it was annoying me. (The specific problem: when I have a bug in my app factory that should cause an immediate crash on startup, Flask swallowed the exception and only showed it to me with the first request.)

So this morning, I dug into the code and learned that this is not a bug, but clearly a deliberate feature added back in 2014 ago. I also learned about the workaround for this feature: flask run --eager-loading. Perfect! Nothing to fix here, right?

So what's the problem? Documentation! I had to dig into the source code to understand what was going on, and to figure out how make Flask behave the way I want. That should be described in the manual.

Expected Behavior

In my perfect world, a bug in the app factory would cause an immediate crash on startup. But since delaying such crashes was clearly added deliberately to Flask, and has been there for 5 years, I do NOT propose changing this behaviour.

What should happen: the next person who is annoyed or confused by this behaviour should find an explanation and a workaround in the docs.

import flask

app = flask.Flask(__nam__)                     # <<< TYPO!!!

@app.route('/')
def index():
    return flask.make_response({"foo": 42}, 200)

Actual Behavior

I google'ed, I searched the manual, and I could not figure out why Flask was making it hard to see my bugs. I had to read the source code to find out about --eager-loading.

Environment

  • Python version: 3.6, 3.7
  • Flask version: 1.1.1
  • Werkzeug version: 0.16.0
@gward gward changed the title Docs: lazy loading delays startup errors, and eager loading is not documented Docs: eager loading is not documented (want to see startup errors at startup) Nov 18, 2019
@ThiefMaster
Copy link
Member

The point of lazy loading is that the dev server DOES NOT CRASH when you save code that has import-time errors (e.g. syntax errors). Usually this is much preferable, because during development you are going to reload the page or otherwise send a request shortly after changing the code anyway... And then you will see the error!

@davidism davidism changed the title Docs: eager loading is not documented (want to see startup errors at startup) eager loading is not documented Nov 18, 2019
@davidism davidism added the docs label Nov 18, 2019
@gward
Copy link
Author

gward commented Nov 18, 2019

Proposed fix: add a section "Startup errors" to page https://flask.palletsprojects.com/en/1.1.x/errorhandling/. I am volunteering to write this documentation, but I'd like some feedback: is this the best place to put it, and is this the right amount of detail?

Quick outline of what I propose to add:

  • flask uses "lazy loading" by default
  • purpose: prevent the dev server from crashing due to import-time errors (after all, you're about to send another request)
  • if you would prefer the dev server to crash, you want "eager loading"
  • just run flask run --eager-loading: problem solved

@davidism
Copy link
Member

Yeah, I'm inclined to leave it as an undocumented option to the run command, it's not intended for general use.

The docs aren't really laid out in a way that documenting every option to every command is easy to keep organized and useful. If you're interested in what options are available, use the --help option to any command or sub-command.

@gward
Copy link
Author

gward commented Nov 18, 2019

The problem is that I had no idea that lazy loading was even a thing until I dug into the source code. Once I knew that, it was not difficult to guess "hey maybe there is a command-line option for this". But it never occurred to me to look for one until I realized this behaviour was deliberate, rather than a bug.

@davidism
Copy link
Member

I'd like to close this in favor of making initial errors immediate. See #3431

@gward
Copy link
Author

gward commented Nov 18, 2019

I'd like to close this in favor of making initial errors immediate. See #3431

Yeah, that would be a big improvement.

In the meantime, I've written the docs -- see #3432. If #3431 is likely to be fixed soon, great! Forget my PR. But if you want to describe the status quo circa 1.1.1, I believe my PR improves things.

@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants