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=b224f841...
Thanks!
--Michael