Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Feature Request] Possibility to configure Dash logger level #1354

Closed
anders-kiaer opened this issue Aug 4, 2020 · 3 comments · Fixed by #1355
Closed

[Feature Request] Possibility to configure Dash logger level #1354

anders-kiaer opened this issue Aug 4, 2020 · 3 comments · Fixed by #1355

Comments

@anders-kiaer
Copy link
Contributor

anders-kiaer commented Aug 4, 2020

Is your feature request related to a problem? Please describe.

These lines,

dash/dash/dash.py

Lines 1626 to 1632 in 7b189ad

self.logger.info("Dash is running on %s://%s%s%s\n", *display_url)
self.logger.info(
" Warning: This is a development server. Do not use app.run_server"
)
self.logger.info(
" in production, use a production WSGI server like gunicorn instead.\n"
)
introduced in #1289, gives by default the following output to the terminal

INFO:     Dash is running on http://127.0.0.1:8050
INFO:     Warning: This is a development server. Do not use app.run_server
INFO:     in production, use a production WSGI server like gunicorn instead.

which could be useful for Python developers that at some point want to take their app from localhost to production/shared server.

However, for other use cases, e.g. when using Dash for building command-line programs with end-user consumption and visualization on localhost only, the messages could for some users be confusing (or even scary 😱🙂), since they are non-programmers and see Warning, Do not together with unknown terminology.

Describe the solution you'd like

The current log level is set here

dash/dash/dash.py

Line 1354 in 7b189ad

self.logger.setLevel(logging.INFO)
in the function

dash/dash/dash.py

Line 1251 in 7b189ad

def enable_dev_tools(
which is called regardless of debug value in app.run_server(debug=False/True).

An easy fix could probably simply be to move

dash/dash/dash.py

Line 1354 in 7b189ad

self.logger.setLevel(logging.INFO)
to where the logger is initialized in the first place,

dash/dash/dash.py

Lines 378 to 379 in 7b189ad

self.logger = logging.getLogger(name)
self.logger.addHandler(logging.StreamHandler(stream=sys.stdout))
since INFO is the final level regardless of debug value. That would enable Python programs using dash to use something like

app = dash.Dash()
app.logger.setLevel(logging.WARNING)

to set appropriate log level before calling app.run_server().

Describe alternatives you've considered

Could also add a new argument to app.run_server or dash.Dash.__init__() with wanted Dash log level.

Additional context

There is debug log potentially printed here

dash/dash/dash.py

Lines 707 to 713 in 7b189ad

self.logger.debug(
"serving -- package: %s[%s] resource: %s => location: %s",
package_name,
package.__version__,
path_in_pkg,
package.__path__,
)
which would then be called during __init__ through init_app():

dash/dash/dash.py

Lines 378 to 386 in 7b189ad

self.logger = logging.getLogger(name)
self.logger.addHandler(logging.StreamHandler(stream=sys.stdout))
if isinstance(plugins, patch_collections_abc("Iterable")):
for plugin in plugins:
plugin.plug(self)
if self.server is not None:
self.init_app()
Is this relying on the user to set root log level to <= DEBUG before running Dash to get these logs? If so, the change mentioned above could still support this simply by moving setLevel to after init_app:

        self.logger = logging.getLogger(name)
        self.logger.addHandler(logging.StreamHandler(stream=sys.stdout))

        if isinstance(plugins, patch_collections_abc("Iterable")):
            for plugin in plugins:
                plugin.plug(self)

        if self.server is not None:
            self.init_app()

        self.logger.setLevel(logging.INFO)
@alexcjohnson
Copy link
Collaborator

We can certainly move the logger.setLevel call, but it actually looks to me as though those two lines can be removed entirely at this point. We added them as part of cleaning up the startup message you see if you don't do anything special when running a standalone dash app. Flask itself displays that message if it thinks it's in production mode so you'll see elsewhere in #1289 we set the FLASK_ENV env var to development - but we discovered that this caused problems with some Flask extensions so we removed it in #1306. That means the default startup message is now redundant:

~/plotly/fiddle> python todo.py
Dash is running on http://127.0.0.1:8050/

 Warning: This is a development server. Do not use app.run_server
 in production, use a production WSGI server like gunicorn instead.

 * Serving Flask app "todo" (lazy loading)
 * Environment: production
   WARNING: This is a development server. Do not use it in a production deployment.
   Use a production WSGI server instead.
 * Debug mode: on

@anders-kiaer
Copy link
Contributor Author

Thanks for the reply @alexcjohnson 🙂

We can certainly move the logger.setLevel call, but it actually looks to me as though those two lines can be removed entirely at this point.

Agree, currently the Dash and Flask messages overlap a bit, so maybe we should simply remove

 Warning: This is a development server. Do not use app.run_server
 in production, use a production WSGI server like gunicorn instead.

as you suggest (I think).

There still perhaps are use cases for configuring the Dash logger level, so maybe we should do both (removing those two lines + moving the setLevel to __init__)? One use case wrt. being able to call setLevel is that when we start the Dash localhost app, we automatically open the URL with webbrowser, reducing the need for printing the full URL. In addition we use a one-time-token added to the URL, making the Dash printed URL simply wrong in our case.


Semi-related: We are silencing the Flask message as well (since that also prints a confusing and not relevant WARNING for the localhost users in our Dash use case). Instead of FLASK_ENV=development we monkey-patch flask during runtime just before dash_app.run_server():

import flask


def silence_flask_startup():
    """Calling this function monkey patches the function flask.cli.show_server_banner
    (https://github.com/pallets/flask/blob/a3f07829ca03bf312b12b3732e917498299fa82d/src/flask/cli.py#L657-L683)
    which by default outputs something like:
     * Serving Flask app "dash_app" (lazy loading)
     * Environment: production
       WARNING: This is a development server. Do not use it in a production deployment.
       Use a production WSGI server instead.
     * Debug mode: off
    This warning is confusing to new users, which thinks something is wrong
    (even though having development/debug mode turned off, and
    limit availability to localhost, is best practice wrt. security).
    After calling this function the exact lines above are not shown
    (all other information/output from the flask instance is untouched).
    """

    def silent_function(*_args, **_kwargs):
        pass

    flask.cli.show_server_banner = silent_function

and then use it something like

silence_flask_startup()
dash_app.run_server(...)

@alexcjohnson
Copy link
Collaborator

Makes sense. Feel like making a PR to both remove that extra log message and move the setLevel?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants