Michael DeHaan wrote: > Javier Palacios wrote: > >> Here is the "final" version of the patch to import debian media. The >> full one is against current git HEAD, while the incremental one should >> be applied after the three previous patches (0,1 and 3). >> >> Only the repo finder part is not modified, because I want to have a >> look to allow creation and management of debian repositories. >> >> As far as I see, only fixing proper ["tree"] is required for >> debian/ubuntu, but I'll not work on that until I'm able to arrange a >> second machine to perform real tests. >> >> Javier Palacios >> >> P.S. : Although they was not intended to, the patches for debian did >> solve issue 221 (cobbler import overwrites distro and profile >> configurations). Within add_entry, if condition 'existing_distro is >> not None', instead of printing "modifying existing distro" message, it >> raises an exception warning about that, so nothing is rewritten. Just >> returning might be goot, and a command line option to force >> overwriting might be required, but the exception at least disables >> unwanted removals. >> ------------------------------------------------------------------------ >> >> >> > > Ok, looked this over, applied it (using the "full" patch), and then > hacked on it a bit: > > Here's some of the changes I made to this, not all related to your > changes. > > I did not add any functions so these fall under the area of bugfixes to > weird corner cases and adding documentation. There is probably more to > do so if others would like to test that would be useful. > > Most of these tests were of the form "import a large amount of trees on > NFS under a common root, whether RHEL 3 or RHEL 4". These were RHEL > trees but should largely cover the CentOS use cases > as well. > > -- I had to add another line to the signature checker to make it see > RHEL 4 trees on NFS again. > -- Added recognition for RHEL 3 Desktop trees (3Desktop) in set_variance > code > -- I had to add "kernel-smp" to the list of kernel RPMs to look for as > it wasn't adding x86_64 architectures as is > -- Changed a lot of "raise" conditions to just warnings so they didn't > stop a large import that contained some trees it didn't yet deal with > (i.e. ppc, s390 (not s390x)). > -- Removed some "raise" lines with code immediately below it that would > never be executed. > -- If a name is used, warn, don't stop, but don't edit it either > (similar to above, which you mentioned) > -- Some extra code formatting (whitespace, etc), added a lot of comments > to methods I originally had uncommented (long time coming from me) > -- converted some module comments from # to docstrings > > I haven't gotten around to testing the Debian/Ubuntu layers yet, the > above was all related to RHEL/Fedora testing. > > General comments > > -- output seems rather "loud" as compared to previously, which was > already loud, I need to make this run quieter, perhaps with a --verbose > mode when debug is required > -- especially over NFS, four "os.path.walk" calls leads to a lot of > os.stat calls underneath -- supporting treeinfo and discinfo should > optomize this further (this was an existing problem too) > > I agree, I think the next thing is definitely setting up the "tree" > variable and showing that we have fully automatic installs straight from > an import and then picking the new distributions from the PXE menus. > Sounds good! > > I realize you don't have the huge library of NFS trees to test against > though hopefully the hard parts are over and all we need to do now is > auto-assign a working tree and have a fully automated template > or two to go with it. That would be pretty slick. (These are also not > the most common import use cases, but are the ones that beat on it the > hardest). > > This is definitely one of the more gnarly areas of code (seeing it has > to be kind of psychic to find things out), so I greatly appreciate your > willingness to work on this. It is also good to see the distro specific > bits abstracted out into different classes. > > Here's my changes on top, a few more are likely coming as I continue > testing this. > > http://git.fedorahosted.org/git/cobbler?p=cobbler;a=commitdiff;h=b224f84192a80648e99cdce89d296487931ce811 > > Thanks! > > --Michael > > > > > > > > > > > > _______________________________________________ > cobbler mailing list > [email protected] > https://fedorahosted.org/mailman/listinfo/cobbler >
Another thought is that if we can break things down into just one os.path.walk (or at least make sure we're not walking the same files twice), that would be a very useful speedup. --Michael _______________________________________________ cobbler mailing list [email protected] https://fedorahosted.org/mailman/listinfo/cobbler
