current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I have had a hard time understanding all the documentation on the logger module and forking processes.

My goal is to have each child process that's worked to write to their own separate log file.

This seems to work but I just want to try and verify that this is the correct way.

import os
import logging


def generate_primes(number):
    logger = logging.getLogger(str(os.getpid()) + ':logger')

    for num in range(number + 1):
       # prime numbers are greater than 1
       if num > 1:
           for i in range(2,num):
               if (num % i) == 0:
                   break
           else:
               logger.info('child:' + str(os.getpid()) + ':' + str(num))

def start_workers():
    logger = logging.getLogger('parent:prime')
    logger.info('parent forking')

    for child in range(10):
        pid = os.fork()

        if not pid:

            logger = logging.getLogger('child:' + str(os.getpid()))
            fh = logging.FileHandler('logs/child_primes_' + str(os.getpid()) + '.log')
            logger.addHandler(fh)

            generate_primes(10000)

            os._exit(0)
        else:
            os.waitpid(pid, 0)


if __name__ == "__main__":

    logging.basicConfig(filename='logs/parent_primes.log', level=logging.INFO)
    logger = logging.getLogger('parent:prime')

    logger.info('Fork parent/child logging example.')
    logger.info('Starting children')

    start_workers()
share|improve this question

2 Answers 2

up vote 3 down vote accepted

Thomas, your solutions looks good. I have few remarks regarding the code style.

  1. The following code looks a bit strange to me:

    for num in range(number + 1):
    # prime numbers are greater than 1
    if num > 1:
        for i in range(2,num):
        ...

    You can easily replace it with:

    for num in range(2, number + 1):
    for i in range(2, num):
    ...

  2. Do not use string concatenations, they are making your code less readable. Better use the .format() method of the string:

    'child:' + str(os.getpid()) + ':' + str(num)

# Better:
'child:{pid}:{num}'.format(pid=os.getpid(), num=num)

  3. In order to not put the meta information to every logging request, you can use logging formatters. Please check that doc for more details: https://docs.python.org/2/howto/logging-cookbook.html#using-logging-in-multiple-modules.

  4. To simplify logging creation and make a code a bit more readable, you can create a function get_logger:

    def get_logger(pid):
    file_name = 'logs/child_primes_{}.log'.format(pid)
    file_handler = logging.FileHandler(file_name)
    name = 'child:{}'.format(pid)
    logger = logging.getLogger(name)
    logger.addHandler(file_handler)
    # TODO: Add a formater here
    return logger
share|improve this answer
up vote 1 down vote

I can see a few things that can be improved:

  • for i in range(2,num): You should have whitespace after the comma: range(2, num)
  • fh: this is a bad name for a file handler: file_handler would be a much better name
  • In the following lines, you have extra lines, that should be removed:
for child in range(10):
    pid = os.fork()

    if pid:

        logger = logging.getLogger(str(os.getpid()) + ':logger')
share|improve this answer
    
Thanks! I updated those lines. I'm looking more to verify that this would work as expected. –  Thomas Schultz Jul 9 at 14:47

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.