From dev-return-35416-apmail-directory-dev-archive=directory.apache.org@directory.apache.org Wed Oct 27 12:37:56 2010 Return-Path: Delivered-To: apmail-directory-dev-archive@www.apache.org Received: (qmail 90091 invoked from network); 27 Oct 2010 12:37:56 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 27 Oct 2010 12:37:56 -0000 Received: (qmail 25387 invoked by uid 500); 27 Oct 2010 12:37:55 -0000 Delivered-To: apmail-directory-dev-archive@directory.apache.org Received: (qmail 25350 invoked by uid 500); 27 Oct 2010 12:37:55 -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 25329 invoked by uid 99); 27 Oct 2010 12:37:55 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 27 Oct 2010 12:37:55 +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:37:51 +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 o9RCbrvj008583; Wed, 27 Oct 2010 12:37:53 GMT Content-Type: multipart/alternative; boundary="===============2511386881950287015==" MIME-Version: 1.0 Subject: Re: Review Request: ApacheDsService.start() method refactoring From: =?utf-8?q?Emmanuel_L=C3=A9charny?= To: =?utf-8?q?directory?= , =?utf-8?q?Kiran_Ayyagari?= , =?utf-8?q?Emmanuel_L=C3=A9charny?= Date: Wed, 27 Oct 2010 12:37:53 -0000 Message-ID: <20101027123753.27681.24493@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> X-Virus-Checked: Checked by ClamAV on apache.org --===============2511386881950287015== 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? > 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( "ldapServer"= ); but this is not where we want to go, I guess... - Emmanuel ----------------------------------------------------------- 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 > = > --===============2511386881950287015== 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?

ATM, the Co=
nfigCreator 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...

- Emmanuel


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
--===============2511386881950287015==--