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

Reply via email to