On Sun, 2011-07-10 at 22:52 -0400, Michael K. Johnson wrote: > Let me start with review on the backend. I'm not commenting on whether or not this belongs upstream, just a brief review of the code. > > I've run through installing both a single archive and multiple > archives of different kinds (tar and cpio) over NFS, HTTP, and FTP. > I have not tested the latest version by burning physical media > with a tarball on it, but that does not exercise any additional > code that has been changed since last time I tested it, so that > should be OK at least for review purposes. > > Note that this code supports xz for decompression; that clearly > requires that xz be included on the media (as it is not in F15, > as I discovered in testing). I'm leaving it there on the theory > that it can be added if anyone cares. Perhaps you could add checks for each of the utilities in your dict so support for a given format is automatically removed when required utilities are not present. pyanaconda.iutil.find_program_in_path can do this for you. > > I don't know if there's a best practice for error handling that > goes beyond raising generic exceptions, for the kinds of "should > not happen" errors that this covers. Let me know if there is an > error case here that it is important to handle some other way, > and please give me a hint of what I should do differently. :) Some kind of error handling/reporting should be present for failure to mount things in the backend class, as well as for failures unpacking archives. You'll generally have an Anaconda instance in those methods, so you can access UI components via anaconda.intf. See places like yuminstall.py for examples. Any time you fail to open or parse a file like your description files something other than raising a python built-in exception needs to happen. Feel free to define some exceptions of your own to raise instead of RuntimeError, but remember that your backend needs to pretty much handle its own failures and/or provide a button to exit/reboot. One last little nit is that there's an extra 'return source' at the end of archiveSource. Dave > > Thanks! > _______________________________________________ > Anaconda-devel-list mailing list > Anaconda-devel-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/anaconda-devel-list _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
