Hello, Denis.

Great news.

> *1. Testing of the cache-based implementation of the service grid.*
> I think, we should make a test suite, that will test the old implementation> 
> until we> remove it from the project.

Aggree. Let's do it.

> *2. DynamicServiceChangeRequest.*
> I think, this class should be splat into two.

Agree. Lets's do it.

> *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other classes> 
> with Services word in them.
> I think, they would look better if we use a singular word *Service *instead.
> Same for *Deployments*.

Personally, I want that names as clearly as possible reflects class content for 
reader.
If we deploy *several* services then it has to be Service*S*.

Same for deployment - if this message will initiate single deployment process 
then it should use deployment.
otherwise - deployments.

So my opinion - it's better to keep current naming.

В Чт, 13/12/2018 в 19:36 +0300, Denis Mekhanikov пишет:
> Guys,
> 
> I've been looking through the PR by Vyacheslav for past few weeks.
> Slava, great job! You've done an impressive amount of work.
> 
> I posted my comments to the PR and had a few calls with Slava.
> I am close to finishing my review.
> There are some points, that I'd like to settle in this discussion to avoid
> controversy.
> 
> *1. Testing of the cache-based implementation of the service grid.*
> I think, we should make a test suite, that will test the old implementation
> until we
> remove it from the project.
> 
> *2. DynamicServiceChangeRequest.*
> I think, this class should be splat into two.
> I don't see any point in having a single class with "*flags"* field, that
> shows, what action it actually represents.
> Usage of *deploy(), markDeploy(...), undeploy(), markUndeploy(...)* looks
> wrong.
> Why not have a separate message type for each action instead?
> 
> *3. Naming.*
> I suggest renaming the following classes:
> *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other classes
> with Services word in them.
> I think, they would look better if we use a singular word *Service *instead.
> Same for *Deployments*.
> I propose the following class names:
> 
> *ServicesDeploymentManager -> ServiceDeploymentManager*
> *ServicesDeploymentActions -> ServiceDeploymentActions*
> *ServicesDeploymentTask -> ServiceDeploymentTask*
> *ServicesCommonDiscoveryData -> ServiceCommonDiscoveryData*
> *ServicesJoinNodeDiscoveryData -> ServiceJoiningNodeDiscoveryData*
> 
> *DynamicServicesChangeRequestBatchMessage -> DynamicServiceChangeRequest*
> *ServicesSingleDeploymentsMessage -> ServiceDeploymentResponse*
> *ServicesFullDeploymentsMessage -> ServiceDeploymentFinishMessage*
> 
> *ServiceSingleDeploymentsResults -> ServiceSingleDeploymentResult*
> *ServiceFullDeploymentsResults -> ServiceFullDeploymentResult*
> 
> Let's do this as the final step of the code review to avoid repeated
> renaming.
> 
> Denis
> 
> чт, 6 дек. 2018 г. в 15:21, Denis Mekhanikov <dmekhani...@gmail.com>:
> 
> > Alexey,
> > 
> > I don't see any problem in letting services work on a deactivated cluster.
> > All services need is discovery messages and compute tasks.
> > Both of these features are available at all times.
> > 
> > But it should be configurable. Services may need caches for their work,
> > so it's better to undeploy such services on cluster deactivation.
> > We may introduce a new property in ServiceConfiguration.
> > 
> > I think, this topic deserves a separate discussion.
> > Could you start another thread?
> > 
> > Denis
> > 
> > чт, 6 дек. 2018 г. в 13:27, Alexey Kuznetsov <akuznet...@apache.org>:
> > 
> > > Hi,   Vyacheslav!
> > > 
> > > I'm thinking about to use Services API to implement Web Agent as a cluster
> > > singleton service.
> > > It will improve Web Console UX, because it will not needed to start
> > > separate java program.
> > > Just start cluster with Web agent enabled on cluster configuration.
> > > 
> > > But in order to do this, I need that services should:
> > >   1) Work when cluster NOT ACTIVE.
> > >   2) Auto restart with cluster (when cluster was restarted).
> > > 
> > > Could we support mentioned features on "Service Grid redesign - phase 2" ?
> > > 
> > > Please let me know.
> > > 
> > > --
> > > Alexey Kuznetsov
> > > 

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to