The history here is we started the project with Flake8 / PEP8 and eventually turned on PyLint through Landscape (which has been a nice service, but is often down, not picking up our PRs)
Unless I'm missing something I think the cost of formatting strings that may not get logged is most likely pretty insignificant. But I could get convinced either way (removing the linting rule, or refactoring the code). Code wins argument. If someone can sign up for doing the task of cleaning this up we'll go in that direction, otherwise I vote for blacklisting the lint rule. btw I can't wait for f-strings! https://www.python.org/dev/peps/pep-0498/ Max On Wed, Oct 26, 2016 at 1:44 PM, George Leslie-Waksman < [email protected]> wrote: > There are a few layers of things going on here. > > The first, and the reason for the landscape warning, is that logger > performs string formatting lazily and doing the formatting explicitly > results in extra processing for log levels that are being ignored. > > The second is that the logger can be configured for % or format style > string formatting and the airflow logger is configured of % style. > > It would be best if we chose a style for logging and then always used that > style, with lazy processing for logging. > > I have seen projects that use % for logging (for log handler compatibility) > and {} for general string formatting. > > --George > > On Mon, Oct 17, 2016 at 8:22 AM Andrew Phillips <[email protected]> > wrote: > > > > My understanding from the latest python documentation is that > > > '.format' supersedes the use of '%'. Therefore, it seemed a little > > > strange that the Landscape configuration was advising the use of '%', > > > > > > I think the thing here is that the "%" that the warning is talking about > > is *not* the same operator that has been superseded by format, although > > it was very likely chosen to look similar, which is confusing. > > > > The warning is not telling the user to go back from: > > > > logging.info("some message with a param {}".format(param)) > > > > to > > > > logging.info("some message with a param %s" % (param)) > > > > but to a different argument passing style: > > > > logging.info("some message with a param %s", param) > > > > Having said that, I'd also be up for disabling this warning if it's > > perceived as more of a nuisance that a useful hint. > > > > Regards > > > > ap > > >
