From 47f461ce4bbfe282233e4c65ecfb2859c4084f99 Mon Sep 17 00:00:00 2001 From: Baptiste Ravier Date: Thu, 23 Apr 2026 14:39:06 +0200 Subject: [PATCH 1/2] scripts: Simplify log level argument handling Let's use builtin `choices` parameter of `argparse` to restrict the set of values the user can choose from: https://docs.python.org/3/library/argparse.html#choices This simplifies the code a bit and doesn't change the behavior. --- src/facturx/scripts/pdfextractxml.py | 25 ++++++++----------------- src/facturx/scripts/pdfgen.py | 25 ++++++++----------------- src/facturx/scripts/webservice.py | 19 +++++++++---------- src/facturx/scripts/xmlcheck.py | 25 ++++++++----------------- 4 files changed, 33 insertions(+), 61 deletions(-) diff --git a/src/facturx/scripts/pdfextractxml.py b/src/facturx/scripts/pdfextractxml.py index cd731c4..1a129ed 100755 --- a/src/facturx/scripts/pdfextractxml.py +++ b/src/facturx/scripts/pdfextractxml.py @@ -19,23 +19,13 @@ def pdfextractxml(args): logger.info( "pdfextractxml version %s using factur-x lib version %s", __version__, fxversion ) - if args.log_level: - log_level = args.log_level.lower() - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - if log_level in log_map: - logger.setLevel(log_map[log_level]) - else: - logger.error( - "Wrong value for log level (%s). Possible values: %s", - log_level, - ", ".join(log_map.keys()), - ) - sys.exit(1) + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + logger.setLevel(log_map[args.log_level]) pdf_filename = args.facturx_orderx_file out_xml_filename = args.xml_file_to_create @@ -88,6 +78,7 @@ def main(args=None): default="info", help="Set log level. Possible values: debug, info, warn, error. " "Default value: info.", + choices=["debug", "info", "warn", "error"], ) parser.add_argument( "-d", diff --git a/src/facturx/scripts/pdfgen.py b/src/facturx/scripts/pdfgen.py index 2f849fd..41b5590 100755 --- a/src/facturx/scripts/pdfgen.py +++ b/src/facturx/scripts/pdfgen.py @@ -19,23 +19,13 @@ def pdfgen(args): logger.info( "pdfgen version %s using factur-x lib version %s", __version__, fxversion ) - if args.log_level: - log_level = args.log_level.lower() - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - if log_level in log_map: - logger.setLevel(log_map[log_level]) - else: - logger.error( - "Wrong value for log level (%s). Possible values: %s", - log_level, - ", ".join(log_map.keys()), - ) - sys.exit(1) + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + logger.setLevel(log_map[args.log_level]) pdf_filename = args.regular_pdf_file output_pdf_filename = args.facturx_orderx_pdf_file @@ -128,6 +118,7 @@ def main(args=None): default="info", help="Set log level. Possible values: debug, info, warn, error. " "Default value: info.", + choices=["debug", "info", "warn", "error"], ) parser.add_argument( "-d", diff --git a/src/facturx/scripts/webservice.py b/src/facturx/scripts/webservice.py index dac7b4e..8f118aa 100755 --- a/src/facturx/scripts/webservice.py +++ b/src/facturx/scripts/webservice.py @@ -125,21 +125,20 @@ def main(args=None): default="info", help="Log level. Possible values: critical, error, warning, " "info (default), debug.", + choices=["critical", "error", "warning", "info", "debug"], ) args = parser.parse_args() if args.logfile: formatter = logging.Formatter("[%(asctime)s] %(levelname)s %(message)s") handler = RotatingFileHandler(args.logfile) - if args.loglevel == "debug": - level = logging.DEBUG - elif args.loglevel == "critical": - level = logging.CRITICAL - elif args.loglevel == "warning": - level = logging.WARNING - elif args.loglevel == "error": - level = logging.ERROR - else: - level = logging.INFO + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warning": logging.WARN, + "error": logging.ERROR, + "critical": logging.CRITICAL, + } + level = log_map[args.loglevel] handler.setLevel(level) handler.setFormatter(formatter) fxlogger.setLevel(level) diff --git a/src/facturx/scripts/xmlcheck.py b/src/facturx/scripts/xmlcheck.py index c673dc9..e664601 100755 --- a/src/facturx/scripts/xmlcheck.py +++ b/src/facturx/scripts/xmlcheck.py @@ -19,23 +19,13 @@ def xmlcheck(args): logger.info( "xmlcheck version %s using factur-x lib version %s", __version__, fxversion ) - if args.log_level: - log_level = args.log_level.lower() - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - if log_level in log_map: - logger.setLevel(log_map[log_level]) - else: - logger.error( - "Wrong value for log level (%s). Possible values: %s", - log_level, - ", ".join(log_map.keys()), - ) - sys.exit(1) + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + logger.setLevel(log_map[args.log_level]) if not isfile(args.xml_file): logger.error("%s is not a filename", args.xml_file) @@ -73,6 +63,7 @@ def main(args=None): default="info", help="Set log level. Possible values: debug, info, warn, error. " "Default value: info.", + choices=["debug", "info", "warn", "error"], ) parser.add_argument( "-f", From 7fa237e8bd3c7262862b5edc22504e9ec9349e93 Mon Sep 17 00:00:00 2001 From: Baptiste Ravier Date: Thu, 23 Apr 2026 16:25:32 +0200 Subject: [PATCH 2/2] Fix library logging Calling `logging.basicConfig()` at import time is an anti-pattern for libraries: it installs a `StreamHandler` on the root logger, causing duplicate log entries in applications that configure their own logging. See Python's documentation "Configuring Logging for a Library": https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library Instead let's allow the library user to set the log level policy and move the scripts specific logging configuration to a dedicated function. --- src/facturx/__init__.py | 17 ++++++++++++++++- src/facturx/facturx.py | 5 +---- src/facturx/scripts/pdfextractxml.py | 18 ++++++++++-------- src/facturx/scripts/pdfgen.py | 18 ++++++++++-------- src/facturx/scripts/webservice.py | 28 ++++++++++++++++------------ src/facturx/scripts/xmlcheck.py | 19 +++++++++++-------- 6 files changed, 64 insertions(+), 41 deletions(-) diff --git a/src/facturx/__init__.py b/src/facturx/__init__.py index 00752dd..fbfefcb 100644 --- a/src/facturx/__init__.py +++ b/src/facturx/__init__.py @@ -1,4 +1,5 @@ -__version__ = "4.2" +import logging + from .facturx import ( generate_from_binary, generate_from_file, @@ -14,6 +15,8 @@ xml_check_xsd, ) +__version__ = "4.2" + __all__ = [ "generate_from_binary", "generate_from_file", @@ -28,3 +31,15 @@ "xml_check_schematron", "xml_check_xsd", ] + +logging.getLogger("factur-x").addHandler(logging.NullHandler()) + + +def configure_script_logging(level=logging.INFO): + logger = logging.getLogger("factur-x") + handler = logging.StreamHandler() + handler.setFormatter(logging.Formatter( + "%(asctime)s [%(levelname)s] %(message)s" + )) + logger.addHandler(handler) + logger.setLevel(level) diff --git a/src/facturx/facturx.py b/src/facturx/facturx.py index c69809e..90b816a 100644 --- a/src/facturx/facturx.py +++ b/src/facturx/facturx.py @@ -55,12 +55,9 @@ import mimetypes import os.path -VERSION = importlib.metadata.version("factur-x") -FORMAT = "%(asctime)s [%(levelname)s] %(message)s" -logging.basicConfig(format=FORMAT) logger = logging.getLogger("factur-x") -logger.setLevel(logging.INFO) +VERSION = importlib.metadata.version("factur-x") FACTURX_FILENAME = "factur-x.xml" ZUGFERD_FILENAMES = ["zugferd-invoice.xml", "ZUGFeRD-invoice.xml"] ORDERX_FILENAME = "order-x.xml" diff --git a/src/facturx/scripts/pdfextractxml.py b/src/facturx/scripts/pdfextractxml.py index 1a129ed..8b9c9ab 100755 --- a/src/facturx/scripts/pdfextractxml.py +++ b/src/facturx/scripts/pdfextractxml.py @@ -8,24 +8,19 @@ from facturx import __version__ as fxversion from facturx import get_xml_from_pdf -from facturx.facturx import logger +from facturx import configure_script_logging __author__ = "Alexis de Lattre " __date__ = "March 2026" __version__ = "0.4" +logger = logging.getLogger("factur-x") + def pdfextractxml(args): logger.info( "pdfextractxml version %s using factur-x lib version %s", __version__, fxversion ) - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - logger.setLevel(log_map[args.log_level]) pdf_filename = args.facturx_orderx_file out_xml_filename = args.xml_file_to_create @@ -102,6 +97,13 @@ def main(args=None): help="Filename of the XML file that will be extracted from the PDF", ) args = parser.parse_args() + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + configure_script_logging(level=log_map[args.log_level]) pdfextractxml(args) diff --git a/src/facturx/scripts/pdfgen.py b/src/facturx/scripts/pdfgen.py index 41b5590..776db9d 100755 --- a/src/facturx/scripts/pdfgen.py +++ b/src/facturx/scripts/pdfgen.py @@ -8,24 +8,19 @@ from facturx import __version__ as fxversion from facturx import generate_from_file -from facturx.facturx import logger +from facturx import configure_script_logging __author__ = "Alexis de Lattre " __date__ = "October 2025" __version__ = "0.9" +logger = logging.getLogger("factur-x") + def pdfgen(args): logger.info( "pdfgen version %s using factur-x lib version %s", __version__, fxversion ) - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - logger.setLevel(log_map[args.log_level]) pdf_filename = args.regular_pdf_file output_pdf_filename = args.facturx_orderx_pdf_file @@ -248,6 +243,13 @@ def main(args=None): help="Optional list of additionnal attachments", ) args = parser.parse_args() + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + configure_script_logging(level=log_map[args.log_level]) pdfgen(args) diff --git a/src/facturx/scripts/webservice.py b/src/facturx/scripts/webservice.py index 8f118aa..5018d00 100755 --- a/src/facturx/scripts/webservice.py +++ b/src/facturx/scripts/webservice.py @@ -24,7 +24,7 @@ from facturx import __version__ as fxversion from facturx import generate_from_file -from facturx.facturx import logger as fxlogger +from facturx import configure_script_logging MAX_ATTACHMENTS = 3 # TODO make it a cmd line option __author__ = "Alexis de Lattre " @@ -32,6 +32,8 @@ __version__ = "0.2" app = Flask(__name__) +logger = logging.getLogger('factur-x') + @app.route("/generate_facturx", methods=["POST"]) def generate_facturx(): @@ -128,24 +130,26 @@ def main(args=None): choices=["critical", "error", "warning", "info", "debug"], ) args = parser.parse_args() + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warning": logging.WARN, + "error": logging.ERROR, + "critical": logging.CRITICAL, + } + level = log_map[args.loglevel] if args.logfile: formatter = logging.Formatter("[%(asctime)s] %(levelname)s %(message)s") handler = RotatingFileHandler(args.logfile) - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warning": logging.WARN, - "error": logging.ERROR, - "critical": logging.CRITICAL, - } - level = log_map[args.loglevel] handler.setLevel(level) handler.setFormatter(formatter) - fxlogger.setLevel(level) - fxlogger.addHandler(handler) + logger.setLevel(level) + logger.addHandler(handler) app.logger.addHandler(handler) app.logger.info("Start webservice to generate Factur-X invoices") - fxlogger.info( + else: + configure_script_logging(level=level) + logger.info( "webservice version %s using factur-x lib version %s", __version__, fxversion ) app.run(debug=args.debug, port=args.port, host=args.host) diff --git a/src/facturx/scripts/xmlcheck.py b/src/facturx/scripts/xmlcheck.py index e664601..0522884 100755 --- a/src/facturx/scripts/xmlcheck.py +++ b/src/facturx/scripts/xmlcheck.py @@ -8,24 +8,20 @@ from facturx import __version__ as fxversion from facturx import xml_check_schematron, xml_check_xsd -from facturx.facturx import logger +from facturx import configure_script_logging __author__ = "Alexis de Lattre " __date__ = "March 2026" __version__ = "0.5" +logger = logging.getLogger("factur-x") + + def xmlcheck(args): logger.info( "xmlcheck version %s using factur-x lib version %s", __version__, fxversion ) - log_map = { - "debug": logging.DEBUG, - "info": logging.INFO, - "warn": logging.WARN, - "error": logging.ERROR, - } - logger.setLevel(log_map[args.log_level]) if not isfile(args.xml_file): logger.error("%s is not a filename", args.xml_file) @@ -89,6 +85,13 @@ def main(args=None): ) parser.add_argument("xml_file", help="Factur-X or Order-X XML file to check") args = parser.parse_args() + log_map = { + "debug": logging.DEBUG, + "info": logging.INFO, + "warn": logging.WARN, + "error": logging.ERROR, + } + configure_script_logging(level=log_map[args.log_level]) xmlcheck(args)