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. 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 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. > 2. One operation required by a GUI is the need to allow the user to > accept a license. How would that be done using this API? Would action > objects be exposed? The GUI must be able to download and get the text > of the license so that it can be shown to the user. > As I said, this is part one of the API, focusing on critical things that can brick your system. There's, as far as I know, no inherent reason that such a function couldn't be added to the API in the future. > 3. Since a GUI displays a list of packages that the user can select, > having the list operation is essential for the GUI. One of the key > operations is taking entries from the list and passing them pack into > the install operation. So it is hard to evaluate the install API > without knowing what list is going to return. > See my above statement. For now list will return exactly what it has, because it is not part of the API. Going a step further, install, right now, takes a list of FMRI's to be installed. I can't imagine an implementation of the API version of list which wouldn't allow it to, somehow, be passed into install. > 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. 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. > 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? > Thanks. > Tom > > > Thanks for the feedback, Brock [snip] _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
