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=...'