On Tue, Sep 01, 2009 at 05:14:02PM -0500, Shawn Walker wrote: >> - line 61 & 112: It looks like you don't remove the temporary file if >> everything goes according to plan? Is this intentional, and if so, >> who cleans up the tempfiles when you're done? > > The idea is that the JSONWriter creates a temporary file to store the > serialized results of the data provided by the caller. Once the > serialization is complete, the caller renames the file into place > wherever they want it using json_writer_obj.pathname. Since this is a > private helper class, that seems reasonable.
Perhaps. The fact that the testsuite leaves tempfile turds everwhere continues to be a source of aggravation. Would you comment the code to state that you expect callers to rename this object if they want to keep its contents? I would really like it if you also added a __del__ method that removes the tempfile if it exits, unrenamed, when the object is destroyed. >> - line 305: Can you explain why you used super() here instead of >> CatalogPartBase? All of the other code that I've seen uses the name >> of the parent class explicitly. > > In case there are further sub-classes in the future, this allows those > to work correctly. super() handles certain patterns of inheritance that > just simply using the base class name would not. Would you provide a bit more detail about the situations where you expect this to be useful? I'm curious what you're doing with this class heirarchy that makes this (possibly) necessary. >> manifest.py: >> particular, it doesn't look like we take any action beyond raising >> an exception if verification fails. Bug 6011 details the need to >> re-download corrupt manifests. If we're able to determine that a >> manifest isn't valid, the transport needs to know that it encoutered >> an error while downloading the manifest, and we should re-download >> the manifest's content; however, I don't see either of these things >> happening here. Is this part of Phase III? > > Yes and no. For the client to get the manifest signature data, the > server has to provide v1 catalogs. So the completion of the plumbing > work will be done in Phase III, but I will be opening a bug for another > enterprising soul to take advantage of the new aqueduct :) Yes, however the step that catches either a MalformedActionError or a MyManifestDidntVerify error ought to be generic. I would suggest that since we're going to the trouble of adding manifest signatures and catalog signatures, we should verify them both too. I'm willing to do the transport side of the work for both of these, but I think we need to be in agreement that this is part of the Phase III deliverables in order for that to happen. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
