Return-Path: Delivered-To: apmail-directory-dev-archive@www.apache.org Received: (qmail 91499 invoked from network); 27 Oct 2010 12:43:36 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 27 Oct 2010 12:43:36 -0000 Received: (qmail 32540 invoked by uid 500); 27 Oct 2010 12:43:36 -0000 Delivered-To: apmail-directory-dev-archive@directory.apache.org Received: (qmail 32378 invoked by uid 500); 27 Oct 2010 12:43:35 -0000 Mailing-List: contact dev-help@directory.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Apache Directory Developers List" Delivered-To: mailing list dev@directory.apache.org Received: (qmail 32363 invoked by uid 99); 27 Oct 2010 12:43:34 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 27 Oct 2010 12:43:34 +0000 X-ASF-Spam-Status: No, hits=-1997.8 required=10.0 tests=ALL_TRUSTED,HTML_MESSAGE X-Spam-Check-By: apache.org Received: from [140.211.11.40] (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 27 Oct 2010 12:43:33 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (8.14.3/8.14.3/Debian-9.1ubuntu1) with ESMTP id o9RChaCB008608; Wed, 27 Oct 2010 12:43:36 GMT Content-Type: multipart/alternative; boundary="===============0361822456647532163==" MIME-Version: 1.0 Subject: Re: Review Request: ApacheDsService.start() method refactoring From: "Kiran Ayyagari" To: =?utf-8?q?directory?= , =?utf-8?q?Kiran_Ayyagari?= , =?utf-8?q?Emmanuel_L=C3=A9charny?= Date: Wed, 27 Oct 2010 12:43:36 -0000 Message-ID: <20101027124336.27680.76371@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org X-ReviewRequest-URL: https://reviews.apache.org/r/14/ In-Reply-To: <20101027113820.27681.35858@reviews.apache.org> References: <20101027113820.27681.35858@reviews.apache.org> --===============0361822456647532163== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On 2010-10-27 04:38:22, Kiran Ayyagari wrote: > > I would suggest that we confine the usage of beans to config reader onl= y(even if they can be accessed publicly), cause we need to copy the propert= y values from bean to the corresponding real instance (e.x DirectoryService= Bean -> 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=C3=A9charny 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 i= n either place. > - As configBean can be used to store partial results (it contains Ads= BaseBeans), you have no way to know which kind of objects you'll get from i= t : 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 plav= e where this bean -> real service object conversion/mapping/creation = > happens. > So instead of we changing the startLdap() method to startLdap( direct= oryServiceBean.getLdapServerBean(), directoryService ); > = > we need a config handler/creator to do this and give us the Directory= Service which we can use directly to start. Am I making sense? > > = > Emmanuel L=C3=A9charny 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( "ldapS= erver" ); > = > 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 ( co= nfigCreator.createDS(directoryServiceBean) ); so if I want to embed the DS I can use config reader and creator and just s= tart the DS like DirectoryService ds =3D configCreator.createDS(directoryServiceBean); ds.startup(); I need not copy the code from startLdap( directoryServiceBean, directorySer= vice ) 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=C3=A9charny 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 =3D instanceLayout.getPartitionsDirectory(); > = > if ( !partitionsDir.exists() ) > { > LOG.info( "partition directory doesn't exist, creating {}", p= artitionsDir.getAbsolutePath() ); > partitionsDir.mkdirs(); > } > = > LOG.info( "using partition dir {}", partitionsDir.getAbsolutePath= () ); > = > initSchemaLdifPartition( instanceLayout ); > initConfigPartition( instanceLayout ); > = > // Read the configuration > cpReader =3D new ConfigPartitionReader( configPartition, partitio= nsDir ); > = > ConfigBean configBean =3D cpReader.readConfig( "ou=3Dconfig" ); > = > DirectoryServiceBean directoryServiceBean =3D configBean.getDirec= toryServiceBean( "default" ); > = > // Initialize the DirectoryService now > DirectoryService directoryService =3D initDirectoryService( insta= nceLayout, directoryServiceBean ); > = > // start the LDAP server > startLdap( directoryServiceBean.getLdapServerBean(), directorySer= vice ); > = > // start the NTP server > startNtp( directoryServiceBean.getNtpServerBean(), directoryServi= ce ); > = > // 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(), directory= Service ); > = > // start the jetty http server > startHttpServer( directoryServiceBean.getHttpServerBean(), direct= oryService ); > } > = > = > Diffs > ----- > = > = > Diff: https://reviews.apache.org/r/14/diff > = > = > Testing > ------- > = > = > Thanks, > = > Emmanuel > = > --===============0361822456647532163== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.o= rg/r/14/

On October 27th, 2010, 4:38 a.m., Kiran Ayy= agari wrote:

I would s=
uggest that we confine the usage of beans to config reader only(even if the=
y can be accessed publicly), cause we need to copy the property values from=
 bean to the corresponding real instance (e.x DirectoryServiceBean -> Di=
rectoryService) and this is better to it in the reader, so we don't nee=
d 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 n=
ow possible and setting up a server gets easier too from a user's pov)<=
/pre>

On October 27th, 2010, 4:52 a.m., Emmanuel L=C3=A9charny wrote:<= /p> 

I'm n=
ot sure this is a good idea :
- if the configBean does the Bean -> real object conversion, then the co=
nfigBean 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 e=
ither place.
- As configBean can be used to store partial results (it contains AdsBaseBe=
ans), 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 co=
nfig hierarchy will be stored the Bean. Not very convenient, IMO
- also the reader reads, it does not instantiate. In Studio, we don't c=
are instantiating anything, so there is no reason to have this part done in=
 the reader. I created a ConfigCreator for this purpose, btw

On October 27th, 2010, 5:20 a.m., Kiran Ayyagari wrote:

Agree wit=
h you, the main thing I want to see is to have a single plave where this be=
an -> real service object conversion/mapping/creation =

happens.
So instead of we changing the startLdap() method to startLdap( directorySer=
viceBean.getLdapServerBean(), directoryService );

we need a config handler/creator to do this and give us the DirectoryServic=
e which we can use directly to start. Am I making sense?

On October 27th, 2010, 5:37 a.m., Emmanuel L=C3=A9charny wrote:<= /p> 

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 u=
se PicoContainer :

startLdap( container, directoryService )

where inside the startLdap we would do :

  LdapServerBean ldapServerBean (LdapServerBean)container.get( "ldapSe=
rver" );

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 ( co=
nfigCreator.createDS(directoryServiceBean) );
so if I want to embed the DS I can use config reader and creator and just s=
tart the DS like
DirectoryService ds =3D configCreator.createDS(directoryServiceBean);
ds.startup();

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

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

- Kiran


On October 27th, 2010, 4:24 a.m., Emmanuel L=C3=A9charny wrote:

Review request for directory.
By Emmanuel L=C3=A9charny.

Updated 2010-10-27 04:24:41

Descripti= on 

I have modified the ApacheDsService start method this way :

    public void start( InstanceLayout instanceLayout ) throws Exception
    {
        File partitionsDir =3D instanceLayout.getPartitionsDirectory();
        =

        if ( !partitionsDir.exists() )
        {
            LOG.info( "partition directory doesn't exist, creating=
 {}", partitionsDir.getAbsolutePath() );
            partitionsDir.mkdirs();
        }

        LOG.info( "using partition dir {}", partitionsDir.getAbso=
lutePath() );
        =

        initSchemaLdifPartition( instanceLayout );
        initConfigPartition( instanceLayout );

        // Read the configuration
        cpReader =3D new ConfigPartitionReader( configPartition, partitions=
Dir );
        =

        ConfigBean configBean =3D cpReader.readConfig( "ou=3Dconfig&qu=
ot; );
        =

        DirectoryServiceBean directoryServiceBean =3D configBean.getDirecto=
ryServiceBean( "default" );
        =

        // Initialize the DirectoryService now
        DirectoryService directoryService =3D initDirectoryService( instanc=
eLayout, directoryServiceBean );

        // start the LDAP server
        startLdap( directoryServiceBean.getLdapServerBean(), directoryServi=
ce );

        // 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(), directorySe=
rvice );

        // start the jetty http server
        startHttpServer( directoryServiceBean.getHttpServerBean(), director=
yService );
    }

Diffs=

View Diff
--===============0361822456647532163==--