brannerchinese/slithersentence — pull request #1 by darius (2013-04-09 03:13:03+00:00)
Title: Suggestions

The commit messages are brief -- I'm happy to chat about why I thought the changes would make it better.

I didn't get to reading downloader.py thoroughly before bedtime. I still could tomorrow.

Some notes I took while going through it, mostly going through the link finder:

testing changes is a bit of a problem for me.
i just run a cycle of both programs to see if it dies horribly,
and that takes a while. also i missed an error i made because
i didn't check the logs.

comment this:
url_core = 'worldjournal'
start_url = 'http://' + url_core + '.com'

generally too broad:
except Exception as e:

add .log and CRAWLED_PAGES to .gitignore

does this mean anything?
20130804_18:05:02_EDT (link_collector.add_links_to_db:94) ERROR:
column url is not unique with URL =
http://worldjournal.com/view/full_news/22184856/article-%E9%A6%96%E7%88%BE%EF%BC%9A%E5%B9%B3%E5%A3%A4%E6%AD%A3%E6%BA%96%E5%82%99%E6%A0%B8%E8%A9%A6

the link_collector is not really used as an object -- it's really just
procedural programming. which is ok but it's a shame python makes you
use 'self.' everywhere.

for example, this method:
def decompress_page(self, file_object):
can do this
self.count_downloaded_pages += 1
though it has nothing to do with decompression

I wonder if we could lighten up on the printing, counting, and
logging, by assuming there's a supervisor process with a separate
stats viewer. note that the exception-catching made me miss a real
error until I got around to reading the logs. (I wrote a crawler for
an image search engine several years ago, could chat about the
pluses and minuses of what I did differently.)

In main() in downloader.py, it checks downloader.urlerrors for the
candidate URLs from the database, but not for the top-level
page. Should it check that too?