Re: [PR] SLING-12417- [Compiled Scripts] Add healthcheck for loading capabilities [sling-org-apache-sling-servlets-resolver]

["raducotescu (via GitHub)" <gi...@apache.org>]

raducotescu commented on code in PR #50:
URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/50#discussion_r1853000481


##########
src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java:
##########
@@ -557,6 +589,23 @@ public void removedBundle(Bundle bundle, BundleEvent event, List<ServiceRegistra
         LOGGER.debug("Bundle {} removed", bundle.getSymbolicName());
         regs.forEach(ServiceRegistration::unregister);
         refreshDispatcher(Collections.emptyList());
+        registeredBundles.remove(bundle.getSymbolicName());
+    }
+    
+    @Override
+    public Result execute() {
+        
+        if (expectedBundles == null) {
+            return new Result(Result.Status.OK,"Healthcheck not configured");

Review Comment:
   ```suggestion
               return new Result(Result.Status.OK,"Health check is not configured.");
   ```



##########
src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java:
##########
@@ -557,6 +589,23 @@ public void removedBundle(Bundle bundle, BundleEvent event, List<ServiceRegistra
         LOGGER.debug("Bundle {} removed", bundle.getSymbolicName());
         regs.forEach(ServiceRegistration::unregister);
         refreshDispatcher(Collections.emptyList());
+        registeredBundles.remove(bundle.getSymbolicName());
+    }
+    
+    @Override
+    public Result execute() {
+        
+        if (expectedBundles == null) {
+            return new Result(Result.Status.OK,"Healthcheck not configured");
+        }
+        
+        if (registeredBundles.containsAll(expectedBundles)) {
+            return new Result(Result.Status.OK,"all expected bundles registered");
+        } else {
+            FormattingResultLog log = new FormattingResultLog();
+            log.warn("Expected bundles : {}, present bundles: {}", expectedBundles, registeredBundles);

Review Comment:
   ```suggestion
               log.warn("Expected bundles : {}, registered bundles: {}", expectedBundles, registeredBundles);
   ```



##########
src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java:
##########
@@ -756,4 +805,16 @@ private Set<BundledRenderUnitCapability> reduce(Set<BundledRenderUnitCapability>
         newSet.addAll(originalCapabilities);
         return newSet;
     }
+
+
+    @ObjectClassDefinition
+    public @interface BundledScriptTrackerConfig {
+        
+        @AttributeDefinition(name="Mandatory Bundles", description="for all of the provided symbolic bundle names the "
+                + "registration process must have been completed successfully for the healthcheck to report ok")
+        String[] mandatoryBundles();

Review Comment:
   ```suggestion
           @AttributeDefinition(name="Mandatory Bundles", description="A list of symbolic bundle names the for which the "
                   + "script registration process must have been successfully completed for the health check to report ok.")
           String[] mandatoryBundles();
   ```



##########
src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java:
##########
@@ -136,10 +154,23 @@ protected void deactivate() {
         if (bt != null) {
             bt.close();
         }
+        if (healthCheckRegistration != null) {
+            healthCheckRegistration.unregister();
+            healthCheckRegistration = null;
+        }
         bundleContext.set(null);
         dispatchers.set(null);
     }
 
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    ServiceRegistration<HealthCheck> registerHealthCheck(String[] tags) {
+        Dictionary props = new Hashtable();
+        props.put(HealthCheck.NAME, "Sightly BundledScriptTracker Healthcheck");

Review Comment:
   This health check doesn't have anything to do with Sightly/HTL.



##########
src/main/java/org/apache/sling/servlets/resolver/internal/bundle/BundledScriptTracker.java:
##########
@@ -557,6 +589,23 @@ public void removedBundle(Bundle bundle, BundleEvent event, List<ServiceRegistra
         LOGGER.debug("Bundle {} removed", bundle.getSymbolicName());
         regs.forEach(ServiceRegistration::unregister);
         refreshDispatcher(Collections.emptyList());
+        registeredBundles.remove(bundle.getSymbolicName());
+    }
+    
+    @Override
+    public Result execute() {
+        
+        if (expectedBundles == null) {
+            return new Result(Result.Status.OK,"Healthcheck not configured");
+        }
+        
+        if (registeredBundles.containsAll(expectedBundles)) {
+            return new Result(Result.Status.OK,"all expected bundles registered");

Review Comment:
   ```suggestion
               return new Result(Result.Status.OK,"All expected bundles have registered their scripts.");
   ```



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

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