Hi Jean, Here are my comments:
General Comments: --------------------------------- - In each of the object types for the 3 different transfer types discussed in the document, there are attributes that will get used by every single operation within that object type, like the "destination directory". There are attributes that will get used only by certain operations ONCE. For those attributes, I don't think they need to be kept as attributes for the object at all. They can just be passed in as arguments when a certain operation is called. I think the "transfer()" method for IPS will be much cleaner if we adopt this philosophy. - There's no discussion about how, if any progress will be reported back to the caller of the transfer module. Does the caller pass in a callback function? - There's no discussion about how to interrupt a transfer-in-progress. This functionality is required when people choose to quit the installer in the middle of a transfer, which could be a long process. Specific Comments: ---------------------------------- section 2: - "The checkpointing module will be available" is one of the dependencies. I do not see how checkpointing is being used in the transfer module. Can you elaborate when and where it will be used? section 3.4: - General comment for all the classes. You don't have a constructor method that talks about what options are required when the object is created. I think it would be very useful to list all the required/optional arguments to the constructor method. section 3.4.1.2.3: - The second sentence is not finished. section 3.4.1.2.4: - "Default value is None" is not quiet correct, I think. If _type is FILELIST, this value is required. If _type is not FILELIST, then, this value is not relevant. section 3.4.1.2.5: - Same comment as above for the "Default value is None" statement. section 3.4.2: - What about mirrors for the publishers, how do they get specified? - You kinda mentioned that IPS APIs will be used for implementing the functionalities in the object. It would be good to explicitly spell it out in this section. Perhaps also discuss why you choose to use IPS API instead of the pkg(5) commands. section 3.4.2.2: - There is no "destination directory" for the IPSTransfer object. That should be a required attribute, right? section 3.4.2.2.5: - Why we need an alternate root. Isn't that the "destination" directory? If so, I think calling it alternate root is confusing. Furthermore, this should be a required attribute. Default value of "none" won't work. section 3.4.2.3.1: - It would be much easier to read if this information is in a table format, I think. - "PURGE-HISTORY will use....". Need to finish this sentence. - "SEARCH will use the _args....". I am not sure what we are searching for here. Package names? Files inside packages? section 3.4.3.2.1: - While we can map all SVR4 package operations, I don't understand how "INFO, PARAM, and TRANSLATE" can be useful in the transfer module. section 3.4.3.2.2: - Again, do you mean "destination directory" here? Also, default value can not be None. section 4: - Since you have all the APIs defined, it would be useful to actually write some psudo-code for the use cases, instead of just describing what each of the installers are doing. That way, it would be easier to see how the APIs can be used. Thanks, --Karen On 02/22/10 13:32, jeanm wrote: > > Please review the design document for the transfer module of the > Caiman Unified Design project. > It can be accessed via: > http://hub.opensolaris.org/bin/preview/Project+caiman/CUE_docs/CUDTransferModule.odt > > > > I would appreciate feedback by COB Friday February 26th. > > Jean McCormack > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
