[GitHub] [ignite] daradurvs commented on a change in pull request #8431: IGNITE-13633 Fix ServiceDescriptor.serviceClass failure in case of de…

[GitBox <gi...@apache.org>]

daradurvs commented on a change in pull request #8431:
URL: https://github.com/apache/ignite/pull/8431#discussion_r525269266



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java
##########
@@ -61,12 +66,6 @@
         return cfg;
     }
 
-    /** */
-    @BeforeClass

Review comment:
       I think that we shouldn't remove this check because this test is relevant only for the new Service Grid implementation.
   We can remove such checks with the legacy SG-processor in future.

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceInfo.java
##########
@@ -37,6 +39,9 @@
     /** */
     private static final long serialVersionUID = 0L;
 
+    /** Context. */
+    private final GridKernalContext ctx;

Review comment:
       `ServiceInfo` is a serializable unit.
   I'm not sure that there is a sense to transfer `GridKernalContext` instance.
   
   Maybe it's better to make the field transient and initialize it after deserialization.
   What do you sink?

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceInfo.java
##########
@@ -116,15 +125,28 @@ public IgniteUuid serviceId() {
     }
 
     /** {@inheritDoc} */
-    @SuppressWarnings("unchecked")
     @Override public Class<? extends Service> serviceClass() {
         if (cfg instanceof LazyServiceConfiguration) {
+            if (srvcCls != null)
+                return srvcCls;
+
             String clsName = ((LazyServiceConfiguration)cfg).serviceClassName();
 
             try {
-                return (Class<? extends Service>)Class.forName(clsName);
+                srvcCls = (Class<? extends Service>)Class.forName(clsName);
+
+                return srvcCls;
             }
             catch (ClassNotFoundException e) {
+                GridDeployment srvcDep = ctx.deploy().getDeployment(clsName);

Review comment:
       We have similar logic here `IgniteServiceProcessor#copyAndInject`
   
   Can't we reuse it?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org