[Python] Looking for feedback on my script please

I think the code is good but if I had to really try to find issues:

  • I prefer less whitespace. Too many blank lines can make it harder to read.

  • You should be aware that these lines can throw more than just KeyError. AttributeError is possible if there is no issue when getting the cookie but the regex fails.

  • Along with the above point these lines could be changed to something like:

    try:
        return re.search('pageID":([0-9]+)', resp.text).group(1)
    except AttributeError:
        raise Exception('Page ID not found for page: %s' % page)
    
  • You may want to write some custom Exceptions to make errors a little more explicit. It's as simple as:

    class FacebookLoginError(Exception):
        pass
    

    then in use: raise FacebookLoginError("Login Failed")

  • requests has a built in method for working with json. These lines can become response = requests.get(...).json()

  • 268-278: If every caption contains a url and is skipped, comment will never be assigned and you will throw a NameError. Same if there are no captions.

    This is a good use for else with your for loop. The else will execute if the for loop never breaks.

    for caption in reversed(j['data']['captions']):
        if any(s in caption['caption'] for s in ['http', 'www']):
            continue
        comment = caption['caption']
        break
    else:
        comment = ''
    
  • 295-299 A catch-all exception can be dangerous. I think it's ok for this app. It would be tedious catching all the IndexErrors and KeyErrors.

  • I see you import logging but your TerminalLogger is a function containing a print statement.

    While that's perfectly fine, you may be interested in knowing that the logging module does support output to console as well as formatting.

TL;DR I think your code is good, here's some extra info about logging


Here's a snippet from a logger I used in a desktop app:

import logging
import logging.handlers
import time
import os


log_directory = os.path.join(os.path.dirname(__file__), "logs\\")

default = time.strftime("%Y%m%d") + '.txt'

logging.basicConfig(level=logging.DEBUG,
                    format='%(asctime)s: %(levelname)-8s: %(name)-12s - %(message)s',
                    datefmt='%Y_%m_%d %H:%M:%S',
                    filename=os.path.join(log_directory, 'debug\\', default),
                    filemode='a')

# Error handler for root logger
eh = logging.handlers.RotatingFileHandler(log_directory + 'errors.txt', maxBytes=70000, backupCount=5)
eh.setLevel(logging.ERROR)
eh.setFormatter(logging.Formatter(
    '%(asctime)s: %(levelname)-8s - %(name)-12s -- Function: %(funcName)-12s -- Line: %(lineno)d -- %(message)s'))
logging.getLogger('').addHandler(eh)


def setup_logger(name, file_name):
    logger = logging.getLogger(name)
    logger.setLevel(logging.DEBUG)
    # create file handler which logs even debug messages
    fh = logging.FileHandler(log_directory + file_name)
    fh.setLevel(logging.DEBUG)
    # create console handler with a higher log level
    console = logging.StreamHandler()
    console.setLevel(logging.INFO)
    # create formatter and add it to the handlers
    fh.setFormatter(logging.Formatter('%(asctime)-20s: %(levelname)-8s: -- %(name)-12s -- %(message)-12s'))
    console.setFormatter(logging.Formatter('%(levelname)-8s: %(name)-12s:  %(message)s'))
    # add the handlers to the logger
    logger.addHandler(fh)
    logger.addHandler(console)
    return logger

Overkill for your app, but it shows some of what you can do with logging. I have the main/default logger set to DEBUG along with its target location, date format, and output format.

I then create a separate handler just for logging ERROR level messages. This uses a RotatingFileHandler which will limit the max size of the error output file and generate a limited number of "backups" and cycle through them as time goes on.

Then I have a function to create individual loggers which will have their own name and file, and output to console as well as the log file.


OK so how would this work with your code?

It would look something like this:

logging.basicConfig(filename='imgur2fb.log',
                    format='[%(asctime)s] %(message)s',
                    datefmt='%Y-%m-%d %H:%M:%S',
                    level=logging.DEBUG)
console = logging.StreamHandler()
console.setLevel(logging.INFO)
logging.getLogger('').addHandler(console)

Then, to use you replace terminalLogger(...) with logging.info(...)

note Though, this will also log info events to your output file because debug is lower level than info. You could always create a separate logger for either file or console output, and use the main logger as the other. Additional note: the main logger writes to console by default but becomes a file handler when you include filename=...'

/r/codereview Thread