Tom Mueller (plain-text) wrote:
Shawn Walker wrote:
webrev:
http://cr.opensolaris.org/~swalker/pkg-cat-p2/
I haven't done a thorough review, but here are some issues that jumped
out at me:
1. The ImageInterface.__init__ method should not take an Image as an
argument. This has the effect of exposing the entire Image object to
the users of ImageInterface, and the original intent of introducing the
api module and ImageInterface was to hide the Image object. Eventually,
client.py should be written without referencing Image at all. Is this a
temporary change?
Adding the 'img' parameter to the __init__ doesn't expose the Image
object to users of the ImageInterface at all.
The Image object was already exposed via the 'img' property that has
existed for a long time now (which I'm working hard to remove consumers of).
But yes, the long-term goal is to not expose the Image object to
consumers at all. This is just a historical artifact because the
current client API is incomplete.
2. The version change in api.py indicates that this new code is
compatible with version 19, but with all of the exceptions that were
removed from api_errors.py, I don't see how this can be compatible.
True, I had forgotten I removed them.
3. I was surprised to see image.py importing pkg.server.catalog.
Shouldn't the client and server code be kept separate? If there is
common code, that should be refactored to the pkg module or in another
shared module.
This is an artifact of the old module architecture. The
pkg.server.catalog module is the only module left that understands v0
catalogs and is used by the server and by the client. I could rename
it, but there seemed little point to that as its life-span is limited to
this release. It will be removed after the 2010.02 release, at which
point the server/client will only support v1 catalogs.
That's why the new catalog code is all in pkg.catalog, because the
client and server will finally use the same catalog code.
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
