On Tue, 2015-08-25 at 16:22 +0200, Jan Cholasta wrote:
> On 25.8.2015 16:09, Simo Sorce wrote:
> > On Tue, 2015-08-25 at 15:52 +0200, Jan Cholasta wrote:
> >> On 25.8.2015 15:37, Simo Sorce wrote:
> >>> On Tue, 2015-08-25 at 11:34 +0200, Jan Cholasta wrote:
> >>>> On 25.8.2015 11:25, Jan Cholasta wrote:
> >>>>> On 24.8.2015 19:51, Simo Sorce wrote:
> >>>>>> Why do we have cainstance.py and ca.py and krainstance.py and kra.py in
> >>>>>> ipaserver/install when you always need both files to do anything around
> >>>>>> installation of the ca ?
> >>>>>>
> >>>>>> Is there a motivation ?
> >>>>>> Or can I simply provide a patch to remove the ca.py and kra.py files an
> >>>>>> unify all code in the proper *instance.py file ?
> >>>>>
> >>>>> ca.py and kra.py are the proper files ready to be migrated to the new
> >>>>> install framework, cainstance.py and krainstance.py will be removed.
> >>>>
> >>>> ... once the migration is done. (Hit send button too fast.)
> >>>>
> >>>> The motivation is that *instance.py do not provide a uniform interface,
> >>>> have a lot of redundant and duplicate stuff and are generally unfit for
> >>>> any further extension.
> >>>
> >>> I have been changing only the instance files, so we are going in
> >>> different directions.
> >>
> >> I don't see how, {ca,kra}instance.py code is called from {ca,kra}.py.
> >> {ca,kra}.py contains all the code that is actually needed to install
> >> CA/KRA that is not in {ca,kra}instance.py and was previously scattered
> >> around ipa-{server,replica,ca,kra}-install.
> >>
> >>>
> >>> I do not really care what file we are going into, but there is a lot of
> >>> code in the installer now that does not tell the user a step is being
> >>> done, while instances do that through the step interface.
> >>
> >> This code was always there, we only moved it to a single location. See
> >> git history.
> >>
> >>>
> >>> The step interface is also a very good way to let someone that read the
> >>> code see what is going on and follow each step.
> >>>
> >>> Are you proposing to stop going through the instances steps ? I found
> >>> the current way kra and ca installation is setup basically a regression,
> >>> it took me a *lot* longer that it should be needed to follow through all
> >>> the steps that are really taken.
> >>
> >> Again, we only moved the code around, and I don't think we created any
> >> regressions.
> >
> > Ok, so the plan is just to move the CAInstance class and code *as is*
> > from cainstance.py to ca.py ?I guess I am confused about what is your
> > plan around this exactly.
> 
> The code in CAInstance will be gradually migrated to a new class in 
> ca.py which uses the new install framework. We started with the 
> top-level ipa-server-install and ipa-replica-install code, which is 
> still not done, but you can see it for reference on how it will look 
> like (ipaserver/install/server/*.py, especially the classes at the 
> bottom of the files).

Well I had to modify server/replicainstall.py quite radically, I had to
create a new set of promote_check and promote functions there. So we are
back to duplicated code, sorry (no way around it).

However the functions in server/replicainstall.py still use the
instances mostly for all components *except* for kra and ca where there
is really confusing code calling unnecessarily instances multiple times
and fooling around with stuff. I do not really understand what you mean
by migrating from CAInstance to what's in server/*install.py because in
there all instances are used for the individual components, so I am now
scratching my head.

The code in ca.py especially is really confusing, I rewrote it once to
eliminate duplications then decided to simply not touch it in my branch
(and threw away the modifications) because it is too confusing and I did
not want to risk regressions. So I created a simple create_replica()
function in the CA instance that does all that is needed.

I guess I will just keep ignoring the confusion and try to come up with
something that "works" but I'd really like to understand what is the
endgame there. If you want to replace instances (why?), what will you
replace it with ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to