distributed icon indicating copy to clipboard operation
distributed copied to clipboard

Do not initialize logging on import

Open fjetter opened this issue 1 year ago • 4 comments
trafficstars

Initializing logging on import has many unwanted side-effects.

Most importantly, it is very difficult to configure / overwrite anything unless the config file is overwritten directly. This should delay log configuration until it is needed. This allows code like this to work

import logging

import dask
import dask.bag as db

from dask.distributed import Client

with dask.config.set({
    "logging": {
        "custom": "info",
        "distributed": "warning",  # Default is INFO which is a little verbose
    }
}):
    client = Client(silence_logs=False)

logger = logging.getLogger("custom")
logger.setLevel(logging.INFO)


def task(n: int):
    logger.info(f"Hello {n}")
    
bag = db.from_sequence([1,2,3])
bag.map(task).compute()

which just prints

2024-05-06 12:55:07,779 - matt - INFO - Hello 1
2024-05-06 12:55:07,779 - matt - INFO - Hello 3
2024-05-06 12:55:07,779 - matt - INFO - Hello 2

fjetter avatar May 06 '24 10:05 fjetter

I likely have to go through a couple of edge cases and adjust some tests for this to work. I haven't tried how the silence_logs kwarg factors in but overall I think this change is a strictly positive improvement

fjetter avatar May 06 '24 11:05 fjetter

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    25 files  ±0      25 suites  ±0   10h 22m 16s :stopwatch: + 5m 23s  4 129 tests ±0   4 013 :white_check_mark:  - 5    110 :zzz: ±0  6 :x: +5  47 698 runs  ±0  45 596 :white_check_mark:  - 6  2 096 :zzz: +1  6 :x: +5 

For more details on these failures, see this check.

Results for commit b2b4a620. ± Comparison against base commit 36020d6a.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 06 '24 11:05 github-actions[bot]

I checked the behavior again. If silence_logs=True we're still setting all the loggers to silent but with silent_logs=False it is respecting the config set in the dask.config ctx manager. I think this behavior makes sense.

fjetter avatar Aug 16 '24 09:08 fjetter

Sorry for doing the annoying thing and chiming in with a useless comment, but this is a continuous source of frustration when you're building Bokeh/Panel apps which import Dask. So we'd really love to see this merged.

philippjfr avatar Oct 03 '24 16:10 philippjfr