Brock,
Sorry for the delay in responding. Please see inline.
Brock Pytlik wrote:
Tom Mueller (pkg-discuss) wrote:
Brock,
Please consider these initial comments.
1. I like the idea of an object-oriented API, i.e., a main object
called Image (a noun describing what is being manipulated) as opposed
to an API name like PkgClient. That's one thing that is nice about
the API as it exists today. Is there a reason why the existing API
can't just be enhanced?
Yes, we don't have an API to enchance.
I disagree, but I don't think that really matters.
The important point that I want to stress is that by the time you get
through the three stages that you've outlined in other parts of this
thread, you'll have ended up creating an API (modules and classes) that
is pretty much as complex as the set of modules and classes that we have
now, except that they will have different names.
Image -> PkgClient
ImagePlan -> PlanDescription
PkgFmri -> PackageDescription
etc.
After phase 3, there will be mostly a 1:1 correspondence between the API
classes and the classes that we have now.
It isn't possible to avoid the complexity of the object module by just
flattening it into fewer classes.
I'd really like to see if the API can be improved by just using the
existing classes, adding underscores to methods that are supposed to be
private, adding additional methods to be able to move implementation
logic out of client.py, move UI stuff out of the classes (anyplace that
calls msg()), improve exception handling, and clearly documenting those
classes and methods that are supposed to be used.
We have an image class which exposes many, many implementation
details, private functions and data members, and needless complexity
to a client. The simple goal is to have all (eventually) user
interface code (CLI/GUI/curses/other options) pass through the same
API. That means that beyond knowing that something called an Image
exists, and is related to a particular directory, the user interface
code should have no knowledge of what an image is or how it's
implemented. I think the problems we as a team have had keeping the
CLI and GUI synced up demonstrates the need for a much stricter
separation between interface and implementation.
To be overly clear, right now client.py imports (among other things):
import pkg.client.image as image
import pkg.client.imageplan as imageplan
import pkg.client.filelist as filelist
import pkg.client.bootenv as bootenv
import pkg.fmri as fmri
import pkg.version
packagemanager.py imports:
import pkg.client.image as image
import pkg.portable as portable
installupdate.py imports:
import pkg.client.bootenv as bootenv
import pkg.client.imageplan as imageplan
import pkg.fmri as fmri
import pkg.indexer as indexer
import pkg.search_errors as search_errors
and remove.py imports:
import pkg.client.bootenv as bootenv
import pkg.client.imageplan as imageplan
All of those imports should be (eventually) replaced with:
import pkg.client.api
import pkg.client.api_errors
Reducing the number of imports (if that would even happen) does not
reduce the number of classes that need to be used to use the API.
Places where that cannot happen likely demonstrate failings in the
API. (fmri and version are the only pieces I'm not certain about. It's
possible we might choose to expose these to the world. But that is a
discussion which can happen far down the road, once we get the
critical issues designed, implemented, and incorporated.)
Along with removing the duplicated code, this should drastically
simplify our lives as all changes which require large amounts of
notification will be localized inside two files, instead of being
splashed across the entire code-base.
[snip]
4. A key question for this API, IMHO, is whether an image plan can
exist independently from the image. Right now, the Image object has
an imageplan member. However, another option is that one or more
image plans could exist at any given time, with each of them pointing
back at the image. This way, plan_install would return an image plan,
and then one would do the prepare, execute, etc. on the image plan,
not the image. I'm not sure what the use case would be for having
multiple image plans active at one time, so maybe that is adding
extra complexity to the API that shouldn't be there.
An image can exist without an imageplan. An image plan cannot exist
without an image. There's no reason that multiple API objects couldn't
be created, one for each image that you want to manipulate. Or
multiple ones for the same image if you want to try out multiple plans.
Again, to be very clear, returning the image plan is essentially what
the code does now. You're exposing the implementation to the outside
world. The purpose of this API is to a) separate interface from
implementation and b) localize changes which change the outward face
of our code.
The concept of an image plan isn't an implementation detail. A user
interface client for pkg(5) needs to understand the concept of an image
plan, the states that it can go through (license acceptance, download,
execute, etc.). This is exactly what the ImagePlan object provides
right now.
Your initial proposal exposes the image plan in the form of the plan_*,
describe, and execute methods and the PlanDescription object. I don't
see how returning an ImagePlan exposes the implementation anymore than
what was in the original proposal, except that ImagePlan probably has
some methods that should start with an underscore.
It's possible (likely/hopeful) that the small number of us on the IPS
team will not be the only people who ever right a front-end for our
IPS code. So, in order to be able to change the number of arguments
that Image.inventory (for example) uses, we either have to keep that
function private (as this API does) or be prepared to notify everyone
who's building a front-end for our code that this function which they
might or might not be using is changing its interface. I think our
experience over the last 5 months demonstrates how well that works.
Python provides several ways to preserve compatibility of interfaces.
We just need to use them.
5. I don't think the term "Ipkg" should show up in the API
(IpkgOutOfDateException) since that is just what the OpenSolaris
package for this software happens to be called at this time.
I don't really care what the exception is called. It's only raised
when the SUNWipkg package is out of date. Would you like to propose a
better name?
SoftwareOutOfDateException
Thanks.
Tom
begin:vcard
fn:Tom Mueller
n:Mueller;Tom
org:Sun Microsystems, Inc.;SWI Install/Update Software
adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA
email;internet:[EMAIL PROTECTED]
title:Senior Staff Engineer
tel;work:877-250-4011
tel;fax:877-250-4011
tel;home:402-916-9943
x-mozilla-html:TRUE
version:2.1
end:vcard
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
