2008/7/10 Danek Duvall <[EMAIL PROTECTED]>:
> I've posted a new webrev:
>
>    http://cr.opensolaris.org/~dduvall/pkg-newlist2/
>
> And the incremental from the previous:
>
>    http://cr.opensolaris.org/~dduvall/pkg-newlist1-2/

http://cr.opensolaris.org/~dduvall/pkg-newlist2/src/modules/client/image.py.html
==========
 857                                         os.path.join(thedir, v,
d, "installed"))

Use constant here.

Only other comment

Otherwise, I'm satisfied with your changes.

>> >       920 +                self.cache_catalogs()
>> >       921 +
>> >  894  922                  if failed:
>> >  895  923                          raise RuntimeError, (failed, total,
>> > succeeded)
>> >
>> > Is 920 and 921 intentionally before 922/923? Is the thought that we
>> > should cache what we can? Is there any concern that if failed was true
>> > that we would be caching an inconsistent or bad catalog state?
>>
>> That was the thought, but it's possible to get a bad catalog.  I'll take
>> another look at this.
>
> So my thought here is that the catalog recv() methods should be more
> careful to prevent bad data from hitting disk.  Right now, they at least
> collect all data from the network before writing anything to disk, but they
> simply append to the existing file, rather than writing to a new one and
> moving it into place.  If we make that latter change, I don't think there's
> any reason that having cache_catalogs() here will be problematic, while
> moving it after the exception raising will mean that the successfully
> downloaded catalogs won't be cached.

That makes sense and i agree.

Cheers,
-- 
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to