> 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... > > > > Kiran Ayyagari wrote: > >> 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 :) > > > akarasulu wrote: > +1 on ds.startup() after configCreator.createDS(directoryServiceBean) > this makes for a cleaner API. > > Emmanuel Lécharny wrote: > ok, so what about creating everything in the bean builder, and pass the > result to the start methods ? But again, we would like to write : > > startLdap( ldapInstance, dsInstance ), no ? > > And how do we get the ldapInstance, btw? configBuilder.getLdapInstance() ? > > This won't be different from what we currently have... > > akarasulu wrote: > elechary clarified the 3 steps in the startup process on IRC: > > 1). Read the configuration from LDAP configuration partition and build > the configuration bean hierarchy > 2). Using the configuration bean hierarchy build a DirectoryService or > other ApacheDSService instance > 3). Start the instance for the DirectoryService or the ApacheDSService > > My 2 cents say keep this all intuitive and easily inferred by our API > users keeping the amount of manual reading they have to do to just get the > job done to a minimum. That means a clean API. Clean API = less methods, less > overloads, less arguments, less classes and a clear process with clear > intuitive names. Here's some recommendations for the builders working at step > 1 and 2. > > 1). Since we're building a containment tree of configuration objects like > someone would a DOM I would associate this with the builder pattern and name > the base class that does this ConfigurationBeanBuilder. > 2). Since we're building a containment tree of components I would > associate this again with the builder pattern and name the class that does > this DirectoryServiceBuilder. > > > Emmanuel Lécharny wrote: > sorry, I think I see now what you'd like to have : > > ds = beanBuilder.createDS( dsBean ); > ds.startup(); > > ldapServer = beanBuilder.createLdapServer( dsBean ); > ldapServer.startup(); > > ... > > Is that it ? > > akarasulu wrote: > NOTE: so you can have code like this (WARNING: extra verbose for now to > be didactic): > > DirectoryServiceConfigBuilder dsConfigBuilder = new > DirectoryServiceConfigurationBuilder( LdapDN dsConfigDn ); > DirectoryServiceConfig dsConfig = dsConfigBuilder.buildConfig(); > DirectoryService ds = DirectoryServiceBuilder.build( buildConfig ); > ds.startup(); > > akarasulu wrote: > Yeah Emmanuel this is what I was thinking. Except I want to stop using > the "Bean" suffix on classes and identifiers. Why? Because we all know they > are beans. Even the components are beans though and that's ambiguous to me. > Why not just say config instead and it's understood that this is just a value > object (a bean) used for the configuration info. > > BTW one other point. Builders can have some member properties that impact > the way they build. However most often we're not going to deal with any. And > in this case the need to have an instance of a builder is moot I think now > but who knows. So why not just have a configuration builder static method > that builds the config bean hierarchy to have simpler code like so: > > DirectoryServiceConfig dsConfig = DirectoryServiceConfigBuilder.build( > dsConfigDn ); > DirectoryService ds = DirectoryServiceBuilder.build( dsConfig ); > ds.startup(); > > WDYT ?
We have to be able to make a distinction between DirectoryService and DirectoryService<Bean>, otherwise we have a conflict. This is true for *all* the elemnts, except for the config. Why would the ConfigBean be different ? We could thogh name it ConfigContainer... I buy the idea of having static methods. There is no need to create a Builder objetc anyway, we don't store any data into it. - 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é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 > >
