directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kiran Ayyagari" <kayyag...@apache.org>
Subject Re: Review Request: ApacheDsService.start() method refactoring
Date Wed, 27 Oct 2010 12:43:36 GMT


> On 2010-10-27 04:38:22, Kiran Ayyagari wrote:
> > I would suggest that we confine the usage of beans to config reader only(even if
they can be accessed publicly), cause we need to copy the property values from bean to the
corresponding real instance (e.x DirectoryServiceBean -> DirectoryService) and this is
better to it in the reader, so we don't need this copying anywhere else to setup a service
(one particular area I see is in embedded mode, currently in embedded server we are not able
to
> > use external config, but with the new single file LDIF based config it is now possible
and setting up a server gets easier too from a user's pov)
> 
> Emmanuel Lécharny wrote:
>     I'm not sure this is a good idea :
>     - if the configBean does the Bean -> real object conversion, then the configBean
will have to be aware of the real object. In any case, there is a tight relation between config
and entities, and it has to be expressed in either place.
>     - As configBean can be used to store partial results (it contains AdsBaseBeans),
you have no way to know which kind of objects you'll get from it : ot can stores AdirectoryService
beans (and more than one), Servers, etc.
>     - if the service init needs to read the configBean and search for the exact Bean
it has to use, then that mean the service has to know where in the config hierarchy will be
stored the Bean. Not very convenient, IMO
>     - also the reader reads, it does not instantiate. In Studio, we don't care instantiating
anything, so there is no reason to have this part done in the reader. I created a ConfigCreator
for this purpose, btw
> 
> Kiran Ayyagari wrote:
>     Agree with you, the main thing I want to see is to have a single plave where this
bean -> real service object conversion/mapping/creation 
>     happens.
>     So instead of we changing the startLdap() method to startLdap( directoryServiceBean.getLdapServerBean(),
directoryService );
>     
>     we need a config handler/creator to do this and give us the DirectoryService which
we can use directly to start. Am I making sense?
>
> 
> Emmanuel Lécharny wrote:
>     ATM, the ConfigCreator class is doing the conversion. 
>     
>     We can do : startLdap( directoryServiceBean, directoryService )
>     
>     that would be probably better.
>     
>     If we go any further, another approach would be to nae all the beans, and use PicoContainer
:
>     
>     startLdap( container, directoryService )
>     
>     where inside the startLdap we would do :
>     
>       LdapServerBean ldapServerBean (LdapServerBean)container.get( "ldapServer" );
>     
>     but this is not where we want to go, I guess...
>     
>

>> We can do : startLdap( directoryServiceBean, directoryService )
IMHO I want this to be done in the config creator, i.e. like startLdap ( configCreator.createDS(directoryServiceBean)
);
so if I want to embed the DS I can use config reader and creator and just start the DS like
DirectoryService ds = configCreator.createDS(directoryServiceBean);
ds.startup();

I need not copy the code from startLdap( directoryServiceBean, directoryService ) to create
the DS and then start

>> but this is not where we want to go, I guess...
definitely NOT :)


- Kiran


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14/#review5
-----------------------------------------------------------


On 2010-10-27 04:24:41, Emmanuel Lécharny wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14/
> -----------------------------------------------------------
> 
> (Updated 2010-10-27 04:24:41)
> 
> 
> Review request for directory.
> 
> 
> Summary
> -------
> 
> I have modified the ApacheDsService start method this way :
> 
>     public void start( InstanceLayout instanceLayout ) throws Exception
>     {
>         File partitionsDir = instanceLayout.getPartitionsDirectory();
>         
>         if ( !partitionsDir.exists() )
>         {
>             LOG.info( "partition directory doesn't exist, creating {}", partitionsDir.getAbsolutePath()
);
>             partitionsDir.mkdirs();
>         }
> 
>         LOG.info( "using partition dir {}", partitionsDir.getAbsolutePath() );
>         
>         initSchemaLdifPartition( instanceLayout );
>         initConfigPartition( instanceLayout );
> 
>         // Read the configuration
>         cpReader = new ConfigPartitionReader( configPartition, partitionsDir );
>         
>         ConfigBean configBean = cpReader.readConfig( "ou=config" );
>         
>         DirectoryServiceBean directoryServiceBean = configBean.getDirectoryServiceBean(
"default" );
>         
>         // Initialize the DirectoryService now
>         DirectoryService directoryService = initDirectoryService( instanceLayout, directoryServiceBean
);
> 
>         // start the LDAP server
>         startLdap( directoryServiceBean.getLdapServerBean(), directoryService );
> 
>         // start the NTP server
>         startNtp( directoryServiceBean.getNtpServerBean(), directoryService );
> 
>         // Initialize the DNS server (Not ready yet)
>         // initDns( configBean );
> 
>         // Initialize the DHCP server (Not ready yet)
>         // initDhcp( configBean );
> 
>         // start the ChangePwd server (Not ready yet)
>         startChangePwd( directoryServiceBean.getChangePasswordServerBean(), directoryService
);
> 
>         // start the Kerberos server
>         startKerberos( directoryServiceBean.getKdcServerBean(), directoryService );
> 
>         // start the jetty http server
>         startHttpServer( directoryServiceBean.getHttpServerBean(), directoryService );
>     }
> 
> 
> Diffs
> -----
> 
> 
> Diff: https://reviews.apache.org/r/14/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Emmanuel
> 
>


Mime
View raw message