Shawn Walker wrote: > Brock Pytlik wrote: >> The flow of control for the consumer of a PkgClient object is: >> 1) Create an API object. This establishes which Image the API object >> is associated with and assigns a progresstracker (for simplicity) >> 2) Call either plan_install, plan_uninstall, plan_image_update >> 3) Call prepare >> 4) Call execute_plan >> >> Cancel essentially sends you back one step (so if it's doing 3 and >> cancel is sent, the state will return to 2). >> Reset returns the state to just after step 1. > > When canceling or resetting, does that affect the image in any way? > > In other words, if a cancel occurs during the prepare phase, would > that clear the download cache? Either way, it would effect the image. If there were previous downloads stored in the cache then clearing them would remove those, and failing to clear them obviously effects the image. Cancel and reset do not return the on disk representation of the image to their prior state. If that's what's desired, the user should use a snapshot and rollback. I see no reason for the API to offer that level of guarantee. What cancel and reset do is make changes to the state of the (for lack of a better term) API Object.
> >> class PkgClient(object): >> def __init__(self, img_path, version_id, progesstracker, >> cancel_state_callable): >> # Raises VersionException, NoImageException > > I still have reservations about a versioned API. I'm going to make two (hopefully uncontroversial) assertions which hold for the rest of this discussion: A) It's unlikely we'll make the perfect API in our first attempt B) The API shouldn't be designed so that only people on the packaging team can write a front-end to our code Given those assertions, that means that the API will likely change over time. That implies that we'll have to notify people consuming the API of those changes. That suggests that we could fail to notify everyone (despite our best efforts), and that not everyone will manage to sync up with the changes in our API the instant we make them. That means there can exist mismatched versions of the API a front-end is expecting, and the API our back-end is providing. In that situation, it seems to me we have 3 choices. 1) Use python to verify that the API's match up by comparing functions, argument lists, and return types. The code could fail on startup if the API's are mismatched. This, in my opinion, is the ideal solution. I also have absolutely no idea how to implement such a thing in Python. If anyone can suggest a very to statically verify that the API's match up (using reflection for example) I'd be very interested in learning more. 2) Ignore the problem. Let the code work when it can, and die and various, and likely unobvious locations. The positive side of this is that sometimes the code will work. The downside of this is that sometimes the code will work, making testing much more difficult. 3) Use something like the version number to attempt to keep the front-end expectations and back-end API in sync. This means when the API is changed, all consumers are notified because the program dies. More importantly, it's guaranteed to die, and die early (making testing easier). It's an imperfect approximation of option 1 in the absence of a way of actually doing option 1. Note that option 2 can be followed using this method by the front-end API simply bumping its number without examining the actual changes to the API. But if we, or others outside blindly bump their API version, they know the risks being taken. > >> def plan_uninstall(self, pkg_list, recursive_removal): >> # pkg_list: list of packages to remove >> # recursive_removal: whether recursive removal should be >> # applied >> # Returns 1 if it has things to do, 0 for nothing to >> be done >> # Raises imageplan.NonLeafPackageException, >> # image.PlanCreationException > > Wouldn't image.UnmatchedFmriException be possible here too? Probably, I'm sure in general that I haven't covered all the exceptions that can escape from the API. I did the best that I could do in the limited time to document the ones that the CLI catches, as well as introducing others so that they could be caught. > >> def describe(self): >> # Returns None if no plan is ready yet, otherwise >> returns >> # a PlanDescription > > describe_plan? > >> def prepare(self): >> # Downloads the packages to disk. >> # Prepares the indexes for updating. >> # Raises >> search_errors.ProblematicPermissionsIndexException, >> # UnknownPlanTypeException > > prepare_plan? > >> def refresh(self, full_refresh, auths=None): >> # full_refresh: whether to do a full retrieval of the >> catalog >> # from the authority or only update the existing >> catalog >> # auths: a list of authorities to refresh. Passing an >> empty >> # list or using the default value means to >> refresh all >> # known authorities > > refresh_catalogs or refresh_authoritiess for clarity? At this stage, I don't really care what the names are, as long as they're internally consistent and make some degree of sense. I have no objections to the names you suggested. > > One request I have is that I need you to look at the review I posted > for bug 1449 to see how you would tie the history object into this new > API. I don't think you'd need to expose the history API through this > -- just ensure that it gets called appropriately by these methods > internally. I'll take a look. > > Finally, I know you wanted the most critical actions to be part of the > API, but I'd be interested in seeing *all* image-modifying operations > be part of the initial API (e.g. rebuilding search indexes and > changing authorities). > Again, we want to cover the things that can brick your system first. Frankly, I think having refresh in there is overkill, but it wasn't hard to add. For November, I think we should focus on install, uninstall, and image-update, (and refresh I guess) and push all other operations off until after November (unless this goes much faster than I anticipate and hope). The only reason I'd move other operations into this set is if they can somehow brick your system or make using it nearly impossible. Preference is also given to those operations which have repeatedly demonstrated their brittle nature. (For example, image-create should definitely be in this API, but as far as I know, we've had 0 API related bugs with image-create, so it falls into the second round of changes, instead of the first.) The user interface to rebuild the search indexes certainly doesn't fall into this category, as at worst, search will simply not work. It's possible that set-authority might fall into that set. I don't particularly see how, but it's possible. I agree that the other operations which modify the image should be the second set of operations moved into the API. Those that merely provide information about the image should be the third set. The goal for November isn't to have a perfect API. The goal is to a) have an API b) have an API which covers those things that are most dangerous and which have demonstrated that another API related failure is merely a matter of time. Thanks for the feedback, Brock > Cheers, _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
