On Fri, May 15, 2009 at 06:21:44PM -0500, Shawn Walker wrote:

> Danek Duvall wrote:
>> On Wed, May 13, 2009 at 05:19:58PM -0500, Shawn Walker wrote:
>>> http://cr.opensolaris.org/~swalker/pkg-6342/
>>
>> pull.py:
>>
>>   - line 167: why CachedManifest?  Is this to speed up
>>     get_hashes_and_sizes()?
>
> It seemed to be a general benefit to most of the operations, and it reduced 
> memory usage quite a bit.

Okay, great.  Do the cache files get cleaned up?  I wouldn't want them
getting rsynced, say, into a repo, or generally getting carried along into
production somewhere.  That's the only downside I see.

>>   - line 397: I think it'd be useful to have a comment here saying that
>>     we don't bother cleaning up a partially copied file because it'll be
>>     overwritten on the next attempt.  Though I think it'd be better to
>>     clean up partially copied files because someone might not decide to
>>     retry, and then their repo wouldn't just be incomplete, it'd be
>>     broken.
>
> I've changed it to remove the possibly, partially copied file.

Looks okay, though perhaps creating a temporary directory to put the
copies into, and then moving them into place would be a tad safer?

>>   - line 489, 490: getting a repo's catalog seems like a candidate for
>>     some part of the API, so that you can call it regardless of the URI
>>     scheme.
>
> I agree, but that's the transport layer and Krister's working on that. The 
> publication api is only for publication to avoid duplication ;)

Sure.  I think it's debatable whether this is transport or some other
layer, but I certainly didn't mean to suggest you should do something about
it in this wad.

>>   - line 605: shouldn't it be possible to republish to a file:// repo
>>     without having to decompress and recompress?
>
> Either the client has to decompress and recompress, or the server does to 
> generate and/or validate the hash and chash.  At the moment, the 
> publication API does not support compressed publication.  I can file an RFE 
> for that, but that's too much churn at the moment to implement.

Ah, of course.  Okay, never mind, then.

Rest of your v1-v2 changes look fine to me.

Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to