From: Chris Rebert on
On Sun, Jul 11, 2010 at 8:13 PM, The Danny Bos <dannybos(a)gmail.com> wrote:
> Thanks gang,
> I'm gonna paste what I've put together, doesn't seem right. Am I way
> off?
>
> Here's my code.
>  - It goes through a table Item
>  - Matches that Item ID to an API call
>  - Grabs the data, saves it and creates the thumbnail
>  - It dies due to Timeouts and Other baloney, all silly, nothing code
> based.
>
> items = Item.objects.all().filter(cover='').order_by('-reference_id')
> for item in items:
>        url = "http://someaddress.org/books/?issue=%s" % item.reference_id
>
>        url_array = []
>        url_open = urllib.urlopen(url)
>        url_read = url_open.read().decode('utf-8')
>
>        try:
>                url_data = simplejson.loads(url_read)
>                url_array.append(url_data)
>
>                for detail in url_array:

Unless I'm missing something, there's no need for url_array to exist
at all. It starts out empty, you append url_data to it, then you
iterate over it as `detail`; and you don't touch it anywhere else in
the loop. Just s/detail/url_data/ and excise url_array altogether. As
a bonus, there'll be one less level of indentation.

Also, the reason your code doesn't work (currently, it just skips to
the next item upon error) is because you're missing a surrounding
`while True` loop (and associated embedded `break`) to do the retrying
(see my or MRAB's examples).

Additionally, stylistically I'd prefer the try-excepts to cover
smaller and more targeted areas of the code, rather than having one
giant blanket one for the entire loop body; perhaps that's just me
though.

Cheers,
Chris
--
How exactly does one acquire a prenominal "The"?
http://blog.rebertia.com
From: The Danny Bos on


Thanks Chris,

Agreed some of the code is a lot useless, I need to go through that
stuff.
So something like this (apologies for asking for some details, I'm not
good at catching):

items = Item.objects.all().filter(cover='').order_by('-reference_id')
for item in items:
url = "http://someaddress.org/books/?issue=%s" %
item.reference_id
url_array = []
url_open = urllib.urlopen(url)
url_read = url_open.read().decode('utf-8')

while True:
try:
url_data = simplejson.loads(url_read)
url_array.append(url_data)
for detail in url_array:
if detail['artworkUrl']:
cover_url =
detail['artworkUrl'].replace(' ','%20')
cover_open =
urllib.urlretrieve(cover_url)
cover_name = os.path.split(cover_url)
[1]
item.cover.save(cover_name,
File(open(cover_open[0])), save=True)
print "Cover - %s: %s" % (item.number,
url)
else:
print "Missing - %s: %s" %
(item.number, url)
break
except ValueError:
print "Error Processing record: %s: %s" %
(item.reference_id, url)
pass
except IOError:
print "IOError; Retrying..."
pass

print "Done"




On Jul 12, 2:14 pm, Chris Rebert <c...(a)rebertia.com> wrote:
> On Sun, Jul 11, 2010 at 8:13 PM, The Danny Bos <danny...(a)gmail.com> wrote:
>
>
>
>
>
> > Thanks gang,
> > I'm gonna paste what I've put together, doesn't seem right. Am I way
> > off?
>
> > Here's my code.
> >  - It goes through a table Item
> >  - Matches that Item ID to an API call
> >  - Grabs the data, saves it and creates the thumbnail
> >  - It dies due to Timeouts and Other baloney, all silly, nothing code
> > based.
>
> > items = Item.objects.all().filter(cover='').order_by('-reference_id')
> > for item in items:
> >        url = "http://someaddress.org/books/?issue=%s" % item.reference_id
>
> >        url_array = []
> >        url_open = urllib.urlopen(url)
> >        url_read = url_open.read().decode('utf-8')
>
> >        try:
> >                url_data = simplejson.loads(url_read)
> >                url_array.append(url_data)
>
> >                for detail in url_array:
>
> Unless I'm missing something, there's no need for url_array to exist
> at all. It starts out empty, you append url_data to it, then you
> iterate over it as `detail`; and you don't touch it anywhere else in
> the loop. Just s/detail/url_data/ and excise url_array altogether. As
> a bonus, there'll be one less level of indentation.
>
> Also, the reason your code doesn't work (currently, it just skips to
> the next item upon error) is because you're missing a surrounding
> `while True` loop (and associated embedded `break`) to do the retrying
> (see my or MRAB's examples).
>
> Additionally, stylistically I'd prefer the try-excepts to cover
> smaller and more targeted areas of the code, rather than having one
> giant blanket one for the entire loop body; perhaps that's just me
> though.
>
> Cheers,
> Chris
> --
> How exactly does one acquire a prenominal "The"?http://blog.rebertia.com

From: Chris Rebert on
> On Jul 12, 2:14 pm, Chris Rebert <c...(a)rebertia.com> wrote:
>> On Sun, Jul 11, 2010 at 8:13 PM, The Danny Bos <danny...(a)gmail.com> wrote:
>> > Thanks gang,
>> > I'm gonna paste what I've put together, doesn't seem right. Am I way
>> > off?
>>
>> > Here's my code.
>> >  - It goes through a table Item
>> >  - Matches that Item ID to an API call
>> >  - Grabs the data, saves it and creates the thumbnail
>> >  - It dies due to Timeouts and Other baloney, all silly, nothing code
>> > based.
>>
>> > items = Item.objects.all().filter(cover='').order_by('-reference_id')
>> > for item in items:
>> >        url = "http://someaddress.org/books/?issue=%s" % item.reference_id
>>
>> >        url_array = []
>> >        url_open = urllib.urlopen(url)
>> >        url_read = url_open.read().decode('utf-8')
>>
>> >        try:
>> >                url_data = simplejson.loads(url_read)
>> >                url_array.append(url_data)
>>
>> >                for detail in url_array:
>>
>> Unless I'm missing something, there's no need for url_array to exist
>> at all. It starts out empty, you append url_data to it, then you
>> iterate over it as `detail`; and you don't touch it anywhere else in
>> the loop. Just s/detail/url_data/ and excise url_array altogether. As
>> a bonus, there'll be one less level of indentation.
>>
>> Also, the reason your code doesn't work (currently, it just skips to
>> the next item upon error) is because you're missing a surrounding
>> `while True` loop (and associated embedded `break`) to do the retrying
>> (see my or MRAB's examples).
>>
>> Additionally, stylistically I'd prefer the try-excepts to cover
>> smaller and more targeted areas of the code, rather than having one
>> giant blanket one for the entire loop body; perhaps that's just me
>> though.

On Sun, Jul 11, 2010 at 9:20 PM, The Danny Bos <dannybos(a)gmail.com> wrote:
> Thanks Chris,
>
> Agreed some of the code is a lot useless, I need to go through that
> stuff.
> So something like this (apologies for asking for some details, I'm not
> good at catching):
>
> items = Item.objects.all().filter(cover='').order_by('-reference_id')
> for item in items:
>    url = "http://someaddress.org/books/?issue=%s" %
> item.reference_id
>    url_array = []
>    url_open = urllib.urlopen(url)
>    url_read = url_open.read().decode('utf-8')
>
>    while True:
>        try:
<snip lots of code>
>                break
>        except ValueError:
>                print "Error Processing record: %s: %s" %
> (item.reference_id, url)
>                pass
>        except IOError:
>                print "IOError; Retrying..."
>                pass
>
> print "Done"

Yes, correct, that's essentially it, although the `pass` statements
are superfluous, and I would personally put the `break` in a
separate+new else-clause of the try-except for clarity; so the
try-except part of the code would look like:

try:
# lots of code here
except ValueError:
print "Error Processing record: %s: %s" % (item.reference_id, url)
except IOError:
print "IOError; Retrying..."
else:
break

Also, please avoid top-posting in the future;
http://en.wikipedia.org/wiki/Top-posting#Top-posting

Cheers,
Chris
--
http://blog.rebertia.com