Package: python-dput
Version: 1.7~bpo70+1
Severity: normal
Tags: patch

Attached file also does some related cleanup of the logging subsystem
to make it possible for third party applications using python-dput to
manage the logging themselves.

Note that library users still TRACE level stderr logging by default,
but may now override it by configuring it's own handler.

-- System Information:
Debian Release: 7.5
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable'), (225, 'testing')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.12-2-amd64 (SMP w/8 CPU cores)
Locale: LANG=sv_SE.UTF-8, LC_CTYPE=sv_SE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages python-dput depends on:
ii  gnupg          1.4.12-7+deb7u3
ii  python         2.7.5-5
ii  python-debian  0.1.21

Versions of packages python-dput recommends:
ii  debian-keyring      2014.04.25
ii  lintian             2.5.22.1tanglu1~deb7u1
ii  openssh-client      1:6.0p1-4+deb7u1
ii  python-distro-info  0.10
ii  python-paramiko     1.7.7.1-3.1
ii  python-validictory  0.8.3-2~bpo70+1

python-dput suggests no packages.

-- no debconf information
>From ec55e612ae340c6621d83c485d70326fbafa52a1 Mon Sep 17 00:00:00 2001
From: Jon Severinsson <j...@severinsson.net>
Date: Sat, 12 Apr 2014 21:01:02 +0200
Subject: [PATCH] Follow logging best practices in the python-dput library.

Note that library users still TRACE level stderr logging by default, but may now override it by configuring it's own handler.

See https://docs.python.org/2/howto/logging.html#configuring-logging-for-a-library
---
 bin/dcut         |   4 +-
 bin/dput         |   3 +-
 dput/core.py     |  42 +++++++++---------
 dput/uploader.py | 127 +++++++++++++++++++++++++++----------------------------
 4 files changed, 86 insertions(+), 90 deletions(-)

diff --git a/bin/dcut b/bin/dcut
index f3085ba9..252b4d73 100755
--- a/bin/dcut
+++ b/bin/dcut
@@ -99,9 +99,7 @@ args = parser.parse_args()
 if args.config:
     dput.core.DPUT_CONFIG_LOCATIONS[args.config] = 1
 
-if args.debug:
-    dput.core._enable_debugging(args.debug)
-
+dput.core._enable_stderr_log(args.debug)
 
 try:
     upload_command(args)
diff --git a/bin/dput b/bin/dput
index db98e621..09037c6a 100755
--- a/bin/dput
+++ b/bin/dput
@@ -85,8 +85,7 @@ if args.host and args.host.endswith(".changes") and os.path.isfile(args.host):
 if args.config:
     dput.core.DPUT_CONFIG_LOCATIONS[args.config] = 1
 
-if args.debug:
-    dput.core._enable_debugging(args.debug)
+dput.core._enable_stderr_log(args.debug)
 
 try:
     overrides = []
diff --git a/dput/core.py b/dput/core.py
index 0becd9ba..1c168c43 100644
--- a/dput/core.py
+++ b/dput/core.py
@@ -28,7 +28,6 @@ import traceback
 import getpass
 
 import dput.logger
-from logging.handlers import RotatingFileHandler
 
 
 # used for searching for config files. place in order of precedence
@@ -69,41 +68,42 @@ Logger, for general output and stuff.
 
 logger.setLevel(dput.logger.TRACE)
 
-# basic config
-_ch = logging.StreamHandler()
-_ch.setLevel(logging.INFO)
-_formatter = logging.Formatter(
-    '%(message)s')
-_ch.setFormatter(_formatter)
 
-
-def _write_upload_log(logfile, full_log):
+def _enable_upload_log(logfile, full_log):
     upload_log_formatter = logging.Formatter(
         "%(asctime)s - dput[%(process)d]: "
         "%(module)s.%(funcName)s - %(message)s"
     )
-    upload_log_handler = RotatingFileHandler(logfile)
+    upload_log_handler = logging.StreamHandler(stream=logfile)
     upload_log_handler.setFormatter(upload_log_formatter)
     if full_log:
         upload_log_handler.setLevel(logging.DEBUG)
     else:
         upload_log_handler.setLevel(logging.INFO)
     logger.addHandler(upload_log_handler)
+    return upload_log_handler
 
 
-def _enable_debugging(level):
-    _ch = logging.StreamHandler()
-    if level == 1:
-        _ch.setLevel(logging.DEBUG)
-    if level >= 2:
-        _ch.setLevel(dput.logger.TRACE)
-    _formatter = logging.Formatter(
-        '[%(levelname)s] %(created)f: (%(funcName)s) %(message)s')
-    _ch.setFormatter(_formatter)
-    logger.addHandler(_ch)
+def _enable_stderr_log(debuglevel):
+    stderr_log_formatter = logging.Formatter(
+        '[%(levelname)s] %(created)f: (%(funcName)s) %(message)s'
+    )
+    stderr_log_handler = logging.StreamHandler()
+    stderr_log_handler.setFormatter(stderr_log_formatter)
+    if debuglevel >= 2:
+        stderr_log_handler.setLevel(dput.logger.TRACE)
+    elif level >= 1:
+        stderr_log_handler.setLevel(logging.DEBUG)
+    else:
+        stderr_log_handler.setLevel(logging.INFO)
+    logger.addHandler(stderr_log_handler)
+    return stderr_log_handler
 
-logger.addHandler(_ch)
 
+def _close_log(handler):
+    logger.removeHandler(handler)
+    handler.flush()
+    handler.close()
 
 def mangle_sys():
     for root in CONFIG_LOCATIONS:
diff --git a/dput/uploader.py b/dput/uploader.py
index 4589f488..9a4b6b02 100644
--- a/dput/uploader.py
+++ b/dput/uploader.py
@@ -32,7 +32,7 @@ from contextlib import contextmanager
 
 import dput.profile
 from dput.changes import parse_changes_file
-from dput.core import logger, _write_upload_log
+from dput.core import logger, _enable_upload_log, _close_log
 from dput.hook import run_pre_hooks, run_post_hooks
 from dput.util import (run_command, get_obj)
 from dput.overrides import (make_delayed_upload, force_passive_ftp_upload)
@@ -279,77 +279,76 @@ def invoke_dput(changes, args):
     else:
         fqdn = profile['name']
 
-    logfile = determine_logfile(changes, profile, args)
-    tmp_logfile = tempfile.NamedTemporaryFile()
-    if should_write_logfile(args):
-        full_upload_log = profile["full_upload_log"]
-        if args.full_upload_log:
-            full_upload_log = args.full_upload_log
-        _write_upload_log(tmp_logfile.name, full_upload_log)
-
-    if args.delayed:
-        make_delayed_upload(profile, args.delayed)
-
-    if args.simulate:
-        logger.warning("Not uploading for real - dry run")
-
-    if args.passive:
-        force_passive_ftp_upload(profile)
-
-    logger.info("Uploading %s using %s to %s (host: %s; directory: %s)" % (
-        changes.get_package_name(),
-        profile['method'],
-        profile['name'],
-        fqdn,
-        profile['incoming']
-    ))
-
-    if changes.get_changes_file().endswith(".changes"):
-        if 'hooks' in profile:
-            run_pre_hooks(changes, profile)
-        else:
-            logger.trace(profile)
-            logger.warning("No hooks defined in the profile. "
-                           "Not checking upload.")
-
-    # check only is a special case of -s
-    if args.check_only:
-        args.simulate = 1
-
-    with uploader(profile['method'], profile,
-                  simulate=args.simulate) as obj:
+    with tempfile.NamedTemporaryFile() as tmp_logfile:
+        if should_write_logfile(args):
+            full_upload_log = profile["full_upload_log"]
+            if args.full_upload_log:
+                full_upload_log = args.full_upload_log
 
-        if args.check_only:
-            logger.info("Package %s passes all checks" % (
-                changes.get_package_name()
-            ))
-            return
+            upload_log = _enable_upload_log(tmp_logfile, full_upload_log)
 
-        if args.no_upload_log:
-            logger.info("Not writing upload log upon request")
+        if args.delayed:
+            make_delayed_upload(profile, args.delayed)
 
-        files = changes.get_files() + [changes.get_changes_file()]
-        for path in files:
-            logger.info("Uploading %s%s" % (
-                os.path.basename(path),
-                " (simulation)" if args.simulate else ""
-            ))
+        if args.simulate:
+            logger.warning("Not uploading for real - dry run")
 
-            if not args.simulate:
-                obj.upload_file(path)
+        if args.passive:
+            force_passive_ftp_upload(profile)
 
-        if args.simulate:
-            return
+        logger.info("Uploading %s using %s to %s (host: %s; directory: %s)" % (
+            changes.get_package_name(),
+            profile['method'],
+            profile['name'],
+            fqdn,
+            profile['incoming']
+        ))
 
         if changes.get_changes_file().endswith(".changes"):
             if 'hooks' in profile:
-                run_post_hooks(changes, profile)
+                run_pre_hooks(changes, profile)
             else:
                 logger.trace(profile)
                 logger.warning("No hooks defined in the profile. "
-                               "Not post-processing upload.")
-    if should_write_logfile(args):
-        tmp_logfile.flush()
-        shutil.copy(tmp_logfile.name, logfile)
-        #print(tmp_logfile.name)
-        tmp_logfile.close()
+                            "Not checking upload.")
+
+        # check only is a special case of -s
+        if args.check_only:
+            args.simulate = 1
+
+        with uploader(profile['method'], profile,
+                    simulate=args.simulate) as obj:
+
+            if args.check_only:
+                logger.info("Package %s passes all checks" % (
+                    changes.get_package_name()
+                ))
+                return
+
+            if args.no_upload_log:
+                logger.info("Not writing upload log upon request")
+
+            files = changes.get_files() + [changes.get_changes_file()]
+            for path in files:
+                logger.info("Uploading %s%s" % (
+                    os.path.basename(path),
+                    " (simulation)" if args.simulate else ""
+                ))
+
+                if not args.simulate:
+                    obj.upload_file(path)
+
+            if args.simulate:
+                return
+
+            if changes.get_changes_file().endswith(".changes"):
+                if 'hooks' in profile:
+                    run_post_hooks(changes, profile)
+                else:
+                    logger.trace(profile)
+                    logger.warning("No hooks defined in the profile. "
+                                "Not post-processing upload.")
+
+        if should_write_logfile(args):
+            _close_log(upload_log)
+            shutil.copy(tmp_logfile.name, determine_logfile(changes, profile, args))
-- 
2.0.0.rc0

Reply via email to