guilbaults / infiniband-exporter

Prometheus exporter for a Infiniband Fabric
Apache License 2.0
55 stars 22 forks source link

Change lazy formatting in logging functions #41

Closed lramosrocha closed 3 years ago

guilbaults commented 3 years ago

Using %s is a bit the old way of doing things, .format() is a bit more readable when the parameters are named (like on line 254).

F-strings is the latest way to do it, but is only available in python 3.6. I think migrating to f-strings make more sense than going back to %s.

gabrieleiannetti commented 3 years ago

Hi,

agreed, using f-strings is definitely more readable way to go for common cases.

Logging is a corner case, were we can optimize the calls by lazy logging.

Otherwise the passed arguments are always evaluated and new string objects are build each time,
which will have an impact on the performance.

As far as I know there is no lazy logging support for f-string yet.

You can use pylint to validate the code, which will show error W1202:

$ pylint infiniband-exporter.py | grep "W1202"
infiniband-exporter.py:254:25: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:263:28: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:307:26: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:324:30: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:556:30: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:756:22: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
infiniband-exporter.py:760:22: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)

Here is a good description for that error: https://vald-phoenix.github.io/pylint-errors/plerr/errors/logging/W1202.html

guilbaults commented 3 years ago

Thanks for the explanation, I didn't realise this was specific to the logging function.