On Mon, Aug 31, 2009 at 08:55:03PM -0500, Shawn Walker wrote:
> webrev:
> http://cr.opensolaris.org/~swalker/pkg-cat-p2/
catalog.py:
- JSONWriter has no docstrings. It would be great if you could add
some description about what this class and its methods do.
- 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?
- line 228 & 237: Why are you raising a RetrievalError here? It
looks to me like you're loading something off disk, so it would make
sense to raise CatalogNotFound, or similar, instead. My expectation
would be that if we're unable to retrieve the catalog during
transport, the exception for the retrieval failure would be
generated from the transport code.
- 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.
pkgplan.py:
- lines 81, 101, 108: Would you replace these with something other
than a RuntimeError. IIRC, Danek asked that we avoid raising that
exception. Since you're modifying this code, let's have it raise
something sane.
manifest.py:
- Who performs the validation of these manifests' signatures? In
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?
General comments:
- You mentioned that this is going to represent a flag day for
clients. Is there anything that RE needs to do to ensure a smooth
transition here?
- I'm also curious if there's anything that needs to happen on the
deployment side of things. My guess is that it's not an issue for
the short term, but are there any code changes coming that are going
to cause problems if the depot software isn't upgraded promptly?
-j
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
