Hello,
I missed below code review comment earlier. But I have replied in PR. 
https://github.com/apache/fineract-cn-demo-server/pull/3

RegardsViswa
    On ‎Tuesday‎, ‎March‎ ‎27‎, ‎2018‎ ‎11‎:‎20‎:‎12‎ ‎AM‎ ‎EDT, Myrle Krantz 
<my...@apache.org> wrote:  
 
 Hi Viswa,

I've done a bit of looking about, and I believe I was wrong about the licensing 
issue with respect to the eclipse plugin.  If you wish to use it, I believe you 
do not have to do anything else to use it. If  you wish to add it back to your 
pull request feel free.

However your latest change with respect to the initialization of static 
variables isn't clean.  Before can be run multiple times for a class, but the 
static variable initialization should only be done once.

What you really want is to create a rule wrapping the existing rule and to 
check the environment variable from within that rule.

For an example of a rule wrapping another rule you can see here:
https://github.com/apache/fineract-cn-test/blob/develop/src/main/java/org/junit/rules/RunExternalResourceOnce.java

Of course you're logic will be a bit different there.  You'll don't need the 
reference counting.  Instead you'll want to access the system variables from 
within the rule.  You won't be able to just cut and past the call to 
"this.environment.containsProperty" into that since it's static.  So use 
System.getProperty() instead.

Best Regards,
Myrle



On 2018/03/11 23:05:47, Viswa Ramamoorthy <viswaramamoor...@yahoo.com.INVALID> 
wrote: 
>  Hi, Myrtle,
> I can make Docker infrastructure enabled explicitly that would leave embedded 
> ActiveMQ and Eureka default. I will update PR on this front.
> Eclipse plugin is to get  project files for import into an IDE typically 
> Spring STS. When 'eclipse' gradle plugin is included, I could typically run 
> './gradlew eclipse' that would create ".project" and ".classpath" files. 
> After this I can import project and add gradle config to have the project 
> available in IDE for debugging purposes. I typically use Spring STS as IDE 
> which is built on top of Eclipse.
> Regarding second point, I need to check and I will add back.
> One other design aspect that stood out was RSA feature (i.e. system 
> properties like public key, private key etc related to certificate) baked 
> into each service and made mandatory. Any reasons to have it mandatory? Would 
> it not be a deployment addition rather than making it mandatory for service 
> start up? Reason I bring this up is that it makes service rely on a pre-step 
> to acquire certificate keys before launch. Even if it is needed in each 
> service, can it be made optional so development environments do not have to 
> deal with this?
> RegardsViswa Ramamoorthy
> 
>    On ‎Sunday‎, ‎March‎ ‎11‎, ‎2018‎ 
>‎03‎:‎38‎:‎48‎ ‎PM‎ ‎EDT, Myrle Krantz <my...@apache.org> 
>wrote:  
>  
>  Hey Viswa,
> 
> You make some fair points.
> 
> But with respect to micro service architecture, it is possible to get
> deploy in one process, but, by keeping the data for the services
> separate, leave the path open to a "proper" micro-service deployment.
> This is what we need to do if we wish to enable developers to work on
> Fineract CN who can't afford higher end machines.
> 
> I suggest we:
> * add the dockerization, but not make it the default.  This is a
> demo-server and not a "real" deployment.
> * add instructions to the demo-server readme to cover the log and
> debugging problems.
> 
> Beyond that there are two (solveable) problems with your pull request:
> 1.) you're using the eclipse gradle plugin.  It's probably under the
> eclipse license.  The eclipse license is a so-called Category B
> license.  You either need to follow that license's requirements or
> remove the plugin.  I haven't looked closely enough at the plugin to
> learn the benefit you derive from adding it.  If you can explain why
> you want it, I'll help you figure out how to conform to the licensing
> requirements.
> 2.) You've accidentally removed spin down code for ActiveMQ and
> Eureka.  These are both @ClassRules in the original code.  This means
> that their "after" functions are called when the test ends.  By moving
> their spin up from the ClassRule into "before", and not adjusting
> "after", you loose the cleanup code.
> 
> Best Regards,
> Myrle
> 
> 
> On Tue, Mar 6, 2018 at 12:54 AM, Viswa Ramamoorthy
> <viswaramamoor...@yahoo.com.invalid> wrote:
> >  Hello Myrle,
> > Thanks for sharing your thoughts on Dockerization of services.
> > My experience with embedding infrastructure inside application JVM (even 
> > for development purposes) has not been great. In a development, when things 
> > change so much, embedding infrastructure adds additional time to bootstrap 
> > the whole thing when application restarts needed. Having external 
> > infrastructures gives better visibility as well as their failure to start 
> > can be diagnosed better (e.g. a port is not available because another 
> > instance of a infrastructure is already running in the background).
> > With external infrastructures, installation becomes cumbersome if we go 
> > with installation of infrastructure and every one need to follow those 
> > steps to install to get there. My PR is really to solve that part.
> > Some of the complexity, that you alluded to, are really complexity of 
> > design/developing in micro services architecture.
> > Couple of points about logging (that stays within Docker) as well as debug 
> > mode with Docker deployment, are very much solvable with Docker deployment.
> > Regarding high amount of resources needed for deployment, one strategy that 
> > could be looked into is to provide capability to selectively start services 
> > needed for a feature to complete and leave the full deployment to 
> > integration environments.
> > If you looking into collapsing micro-services into a single war, for 
> > development purposes, it can be a strategy that would work. But all of the 
> > services need to be using compatible version of frameworks and managing 
> > different configurations can be challenge.
> > Having infrastructure as Docker can still come handy in day to day 
> > development. I understand the timeline/priority. No problem.
> > RegardsViswa
> >
> >    On ‎Monday‎, ‎March‎ ‎5‎, ‎2018‎ 
> >‎07‎:‎17‎:‎41‎ ‎AM‎ ‎EST, Myrle Krantz <my...@apache.org> 
> >wrote:
> >
> >  Hey Viswa,
> >
> > It's going to take me a little longer to get to merging and reviewing
> > this, so please be patient with me.  But a couple of comments while
> > you're waiting:
> >
> > 1.) That you're not seeing those error messages probably may not mean
> > they are gone.  It may mean that they are now "hidden" in the docker
> > image.  That's not ideal for error messages.  It makes debugging
> > harder when there really is an issue.
> > 2.) Thank you for finding the error with the artifact path.  Consider
> > submitting a patch to fineract-cn-service-starter.
> >
> > I'm a bit concerned about the idea of moving this all into docker.
> > Yes docker is one important method for deploying microservices, and
> > showing an example of how to use those technologies is important.  But
> > the demo-server is also there partly to test code and get a local
> > installation up and running.  When I started on it, my intention was
> > to support Mark van Veen so that he didn't have to start all the
> > services and then provision by hand to work on the UI.  Unfortunately
> > there are serious problems with the demo-server the way it is now.  It
> > takes a huge amount of resources because it starts every service in
> > its own process.  Many developers do not have computers with
> > sufficient resources to run this locally.  At one point, Kuelap
> > literally bought me a new computer after I had spent a couple of days
> > unsuccessfully trying to make the demo-server work because Markus had
> > added a couple more services to it.  Moving these processes into
> > containers doesn't solve this problem.  Docker works with computing
> > resources in a shockingly efficient manner, so it probably doesn't
> > actually make the problem worse, but it does make the problem harder
> > to solve.
> >
> > Another point, is that currently I can start these services in debug
> > mode, and attack a debugger to them to understand tricky problems.  I
> > don't know how to do that in a docker container.  Any changes in that
> > direction should consider this use case.
> >
> > I can see that testing this running in docker might be important for
> > some of our users and contributors.  But I don't want it to be the
> > default.  I would feel more comfortable with your change sets if you
> > made this "more optional".
> >
> > My first priority for this project is to enable contributors.  To do
> > that, I'd like to look for ways to run all of the services in one
> > process for the purposes of local testing and debugging.
> >
> > Best Regards,
> > Myrle Krantz
> > Committer, Apache Fineract
> >
> >
> >
> > On Fri, Mar 2, 2018 at 3:35 AM, Viswa Ramamoorthy
> > <viswaramamoor...@yahoo.com.invalid> wrote:
> >>  Hi,
> >> I  have raised a PR with docker compose yml for Eureka and ActiveMQ.
> >> It is https://github.com/apache/fineract-cn-demo-server/pull/3
> >> Please note that after I launch  Eureka and ActiveMQ via Docker, I do not 
> >> see JMS connect error as well as Eureka registration error anymore.
> >> But service launch was failing with below errorCould not find artifact 
> >> io.mifos.provisioner:service-boot:jar:0.1.0-BUILD-SNAPSHOT
> >> Locally was able to fix artifact path to "org.apache.fineract.cn." in 
> >> fineract-cn-service-starter and move forward with service launch.
> >> But there were more errors. I have not looked into further yet.
> >> I think demo server needs some more work to get it to work consistently. 
> >> All of the services can be launched via shell script if there are no start 
> >> up dependencies between them
> >> Regards
> >> Viswa
> >  
  

Reply via email to