#49853 Issue 49844 - lib389: don't set up logging at module scope
Closed 3 years ago by spichugi. Opened 5 years ago by spichugi.
spichugi/389-ds-base logging_fix  into  master

file modified
+6 -1
@@ -1,5 +1,6 @@ 

- import pytest

  import subprocess

+ import logging

+ import pytest

  

  pkgs = ['389-ds-base', 'nss', 'nspr', 'openldap', 'cyrus-sasl']

  
@@ -28,6 +29,10 @@ 

          request.config._metadata['FIPS'] = is_fips()

  

  

+ def pytest_cmdline_main(config):

+     logging.basicConfig(level=logging.DEBUG)

+ 

+ 

  def pytest_report_header(config):

      header = ""

      for pkg in pkgs:

file modified
+2 -2
@@ -38,7 +38,7 @@ 

  

  from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat

  

- from lib389.cli_base import reset_get_logger

+ from lib389.cli_base import setup_script_logger

  

  parser = argparse.ArgumentParser(allow_abbrev=True)

  parser.add_argument('instance',
@@ -93,7 +93,7 @@ 

  

      defbase = ldap.get_option(ldap.OPT_DEFBASE)

      args = parser.parse_args()

-     log = reset_get_logger('dsconf', args.verbose)

+     log = setup_script_logger('dsconf', args.verbose)

  

      log.debug("The 389 Directory Server Configuration Tool")

      # Leave this comment here: UofA let me take this code with me provided

file modified
+2 -2
@@ -17,7 +17,7 @@ 

  

  from lib389 import DirSrv

  from lib389.cli_ctl import instance as cli_instance

- from lib389.cli_base import reset_get_logger

+ from lib389.cli_base import setup_script_logger

  

  parser = argparse.ArgumentParser()

  parser.add_argument('-v', '--verbose',
@@ -49,7 +49,7 @@ 

  if __name__ == '__main__':

      args = parser.parse_args()

  

-     log = reset_get_logger("dscreate", args.verbose)

+     log = setup_script_logger("dscreate", args.verbose)

  

      log.debug("The 389 Directory Server Creation Tool")

      # Leave this comment here: UofA let me take this code with me provided

file modified
+2 -2
@@ -22,7 +22,7 @@ 

  from lib389 import DirSrv

  from lib389.cli_ctl import instance as cli_instance

  from lib389.cli_ctl import dbtasks as cli_dbtasks

- from lib389.cli_base import disconnect_instance, reset_get_logger

+ from lib389.cli_base import disconnect_instance, setup_script_logger

  

  parser = argparse.ArgumentParser()

  parser.add_argument('-v', '--verbose',
@@ -54,7 +54,7 @@ 

  if __name__ == '__main__':

      args = parser.parse_args()

  

-     log = reset_get_logger('dsctl', args.verbose)

+     log = setup_script_logger('dsctl', args.verbose)

  

      log.debug("The 389 Directory Server Administration Tool")

      # Leave this comment here: UofA let me take this code with me provided

file modified
+2 -5
@@ -27,7 +27,7 @@ 

  from lib389.cli_idm import posixgroup as cli_posixgroup

  from lib389.cli_idm import user as cli_user

  

- from lib389.cli_base import connect_instance, disconnect_instance, reset_get_logger

+ from lib389.cli_base import connect_instance, disconnect_instance, setup_script_logger

  

  from lib389.cli_base.dsrc import dsrc_to_ldap, dsrc_arg_concat

  
@@ -78,12 +78,9 @@ 

  if __name__ == '__main__':

  

      defbase = ldap.get_option(ldap.OPT_DEFBASE)

- 

- 

- 

      args = parser.parse_args()

  

-     log = reset_get_logger('dsidm', args.verbose)

+     log = setup_script_logger('dsidm', args.verbose)

  

      log.debug("The 389 Directory Server Identity Manager")

      # Leave this comment here: UofA let me take this code with me provided

@@ -55,15 +55,8 @@ 

  import uuid

  import json

  from shutil import copy2

- try:

-     # There are too many issues with this on EL7

-     # Out of the box, it's just outright broken ...

-     import six.moves.urllib.request

-     import six.moves.urllib.parse

-     import six.moves.urllib.error

-     import six

- except ImportError:

-     pass

+ import six

+ 

  from ldap.ldapobject import SimpleLDAPObject

  from ldap.cidict import cidict

  from ldap import LDAPError

@@ -21,10 +21,8 @@ 

  

  MAJOR, MINOR, _, _, _ = sys.version_info

  

- logging.basicConfig(level=logging.DEBUG)

  log = logging.getLogger(__name__)

  

- 

  class FormatDict(cidict):

      def __getitem__(self, name):

          if name in self:

@@ -289,18 +289,8 @@ 

      def __len__(self):

          return len(self.__dict__.keys())

  

- log_simple_handler = logging.StreamHandler()

- log_simple_handler.setFormatter(

-     logging.Formatter('%(message)s')

- )

  

- log_verbose_handler = logging.StreamHandler()

- log_verbose_handler.setFormatter(

-     logging.Formatter('%(levelname)s: %(message)s')

- )

- 

- 

- def reset_get_logger(name, verbose=False):

+ def setup_script_logger(name, verbose=False):

      """Reset the python logging system for STDOUT, and attach a new

      console logger with cli expected formatting.

  
@@ -311,21 +301,18 @@ 

      :return: logging.logger

      """

      root = logging.getLogger()

-     if root.handlers:

-         for handler in root.handlers:

-                 root.removeHandler(handler)

- 

-     if verbose:

-         root.addHandler(log_verbose_handler)

-     else:

-         root.addHandler(log_simple_handler)

- 

      log = logging.getLogger(name)

+     log_handler = logging.StreamHandler()

  

      if verbose:

          log.setLevel(logging.DEBUG)

+         log_format = '%(levelname)s: %(message)s'

      else:

          log.setLevel(logging.INFO)

+         log_format = '%(message)s'

+ 

+     log_handler.setFormatter(logging.Formatter(log_format))

+     root.addHandler(log_handler)

  

      return log

  

file modified
+2 -1
@@ -502,7 +502,8 @@ 

          #

  

          # First role and replicaID

-         if (role != ReplicaRole.MASTER and

+         if (

+             role != ReplicaRole.MASTER and

              role != ReplicaRole.HUB and

              role != ReplicaRole.CONSUMER

          ):

@@ -1,7 +1,7 @@ 

  import logging

  import six

+ 

  logging.basicConfig(level=logging.DEBUG)

- log = logging.getLogger(__name__)

  

  DN_RMANAGER = 'uid=rmanager,cn=config'

  

@@ -85,7 +85,6 @@ 

  

  _ds_paths = Paths()

  

- logging.basicConfig(level=logging.DEBUG)

  log = logging.getLogger(__name__)

  

  # Private constants

@@ -53,7 +53,6 @@ 

  

  MAJOR, MINOR, _, _, _ = sys.version_info

  

- logging.basicConfig(level=logging.DEBUG)

  log = logging.getLogger(__name__)

  #

  # Decorator

Bug description: lib389 was calling logging.basicConfig()
at several places at module scope level. This was causing
imports from these modules to add an unwanted handler
to the root logger of the python's standard logging module.

Fix description: Set up logging only in the scripts that are
using lib389.
ALso, remove unused imports in init.py

https://pagure.io/389-ds-base/issue/49844

Reviewed by: spichugi

rebased onto 1dcffa633a1409e94c6b8af891c2911c173f8bd0

5 years ago

rebased onto 7892486

5 years ago

Pull-Request has been merged by spichugi

5 years ago

There was some odd behaviours with logging in python, because it heavily depends on order of module loading. That's why that was put around the place. So it's worth being sure to check that all the cli outputs still work as expected after this change :)

I think it looks okay because of the nature of the change, but keep in mind if you see odd logging behaviour,

Thanks!

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2912

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago