You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by GitBox <gi...@apache.org> on 2022/06/01 08:19:57 UTC

[GitHub] [unomi] jsinovassin opened a new pull request, #432: Unomi 582 graphql as feature flag

jsinovassin opened a new pull request, #432:
URL: https://github.com/apache/unomi/pull/432

   https://issues.apache.org/jira/browse/UNOMI-582


-- 
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@unomi.apache.org

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


[GitHub] [unomi] jkevan merged pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jkevan merged PR #432:
URL: https://github.com/apache/unomi/pull/432


-- 
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@unomi.apache.org

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


[GitHub] [unomi] jkevan commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jkevan commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r891392822


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +269,109 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void prepareGraphQLFeatureToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {

Review Comment:
   ```suggestion
           if (Boolean.parseBoolean(bundleContext.getProperty("org.apache.unomi.graphql.feature.activated"))) {
   ```



##########
package/pom.xml:
##########
@@ -377,9 +377,8 @@
                         <feature>unomi-router-karaf-feature</feature>
                         <feature>unomi-web-tracker-karaf-kar</feature>
                         <feature>unomi-groovy-actions</feature>
-                        <feature>cdp-graphql-feature</feature>
                         <feature>unomi-rest-ui</feature>
-                        <feature>unomi-graphql-ui</feature>
+                        <feature>cdp-graphql-feature-denpendencies</feature>

Review Comment:
   So if I do understand correctly this feature is still boot and all the GraphQL dependencies are still started up even if we have disabled GraphQL ?
   May be this feature should be merged with the unomi graphQL implem feature and be activated by the BundleWatcher.



##########
graphql/karaf-feature/src/main/feature/feature.xml:
##########
@@ -52,6 +53,11 @@
         <bundle start-level="80">mvn:org.eclipse.jetty/jetty-security/${jetty.websocket.version}</bundle>
         <bundle start-level="80">mvn:org.eclipse.jetty/jetty-server/${jetty.websocket.version}</bundle>
         <bundle start-level="80">mvn:org.eclipse.jetty/jetty-http/${jetty.websocket.version}</bundle>
-        <bundle start-level="80" start="false">mvn:org.apache.unomi/cdp-graphql-api-impl/${project.version}</bundle>
+    </feature>
+    <feature name="cdp-graphql-feature" description="Apache Unomi :: GraphQL API :: Karaf Feature"
+             version="${project.version}">
+        <feature>unomi-kar</feature>
+        <bundle start-level="80">mvn:org.apache.unomi/cdp-graphql-api-impl/${project.version}</bundle>
+        <bundle start-level="80">mvn:org.apache.unomi/unomi-graphql-playground/${project.version}</bundle>

Review Comment:
   Why not merging in a single feature ? 
   seem's that one doesnt make sense without the other.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcher.java:
##########
@@ -30,10 +30,12 @@ public interface BundleWatcher {
      * Retrieves the list of the server information objects, that include extensions. Each object includes the
      * name and version of the server, build time information and the event types
      * if recognizes as well as the capabilities supported by the system.
+     *
      * @return a list of ServerInfo objects with all the server information
      */
     List<ServerInfo> getServerInfos();
 
-
     boolean isStartupComplete();
+
+    boolean allAdditionalBundleStarted();

Review Comment:
   IS it necessary to have a public function about that ?
   For what purpose ? 
   If it's necessary, please add some JavaDoc.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +269,109 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void prepareGraphQLFeatureToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
+    private void startSchedulerForAdditionalBundles() {
+        if (scheduledFutureAdditionalBundles == null || scheduledFutureAdditionalBundles.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+                        installingFeatureStarted = true;
+                        installFeatures();
+                    }
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    checkStartupComplete();
+                }
+            };
+            scheduledFutureAdditionalBundles = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }

Review Comment:
   The code of this function could be reduce to the task it self being different. 
   (In case we reuse the same ScheduledFuture variable like suggested below.)
   
   Or other option, just use one function here and pass everything as parameter:
   
   - installFeature: bool -> false first time, true second time
   - inactiveBundlesToWatch: requiredBundles first time, requiredBundlesFromFeatures second time
   - checkInactiveServices: bool -> true first time, false second time
   
   Doing so we can:
   
   - reuse existing ScheduledFuture variable, because both cannot be start in parallel, the feature task is started after the normal bundle task is ended.
   - And we can have a generic task that is able to handle both cases depending on the parameter listed previously.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +269,109 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void prepareGraphQLFeatureToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
+    private void startSchedulerForAdditionalBundles() {
+        if (scheduledFutureAdditionalBundles == null || scheduledFutureAdditionalBundles.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+                        installingFeatureStarted = true;
+                        installFeatures();
+                    }
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    checkStartupComplete();
+                }
+            };
+            scheduledFutureAdditionalBundles = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+
+        if (!allAdditionalBundleStarted()) {
+            startSchedulerForAdditionalBundles();
+            return;
+        }
+        if (scheduledFutureAdditionalBundles != null) {
+            scheduledFutureAdditionalBundles.cancel(true);
+            scheduledFutureAdditionalBundles = null;
+        }

Review Comment:
   Since the previous **scheduledFuture** is finished and destroyed we could reuse it instead of adding a new **scheduledFutureAdditionalBundles** variable ?
   
   And the destroy code (cancel and set to null can be extracted to a function called: **destroyScheduler**)



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jkevan commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jkevan commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888697267


##########
tools/shell-commands/src/main/java/org/apache/unomi/shell/services/internal/UnomiManagementServiceImpl.java:
##########
@@ -31,13 +34,19 @@
  */
 public class UnomiManagementServiceImpl implements UnomiManagementService {
 
+    private static final Logger logger = LoggerFactory.getLogger(UnomiManagementServiceImpl.class);
+
     private BundleContext bundleContext;
+    private FeaturesService featuresService;
     private List<String> bundleSymbolicNames;
     private List<String> reversedBundleSymbolicNames;
 
+    private static final String CDP_GRAPHQL_FEATURE = "cdp-graphql-feature";

Review Comment:
   Seem's not used anymore ?



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }

Review Comment:
   Could we try to generify this using Service property and properties ?
   If not possible it's ok to keep it in the code directly, but may be found a name more meainingful that it's related to GraphQL feature.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }

Review Comment:
   To be removed, this check need to be inside the: isStartupComplete directly.



##########
tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml:
##########
@@ -20,12 +20,14 @@
            xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
            xsi:schemaLocation="http://www.osgi.org/xmlns/blueprint/v1.0.0 http://www.osgi.org/xmlns/blueprint/v1.0.0/blueprint.xsd">
 
+    <reference id="featuresService" interface="org.apache.karaf.features.FeaturesService"/>
+

Review Comment:
   can be cleaned, removed.



##########
tools/shell-commands/src/main/java/org/apache/unomi/shell/services/internal/UnomiManagementServiceImpl.java:
##########
@@ -31,13 +34,19 @@
  */
 public class UnomiManagementServiceImpl implements UnomiManagementService {
 
+    private static final Logger logger = LoggerFactory.getLogger(UnomiManagementServiceImpl.class);
+
     private BundleContext bundleContext;
+    private FeaturesService featuresService;

Review Comment:
   Seem's not used anymore ?



##########
tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml:
##########
@@ -59,13 +61,14 @@
                 <value>org.apache.unomi.router-rest</value>
                 <value>org.apache.unomi.shell-dev-commands</value>
                 <value>org.apache.unomi.web-tracker-wab</value>
-                <value>org.apache.unomi.cdp-graphql-api-impl</value>
                 <value>org.apache.unomi.groovy-actions-services</value>
                 <value>org.apache.unomi.groovy-actions-rest</value>
             </list>
         </property>
         <property name="bundleContext" ref="blueprintBundleContext"/>
+        <property name="featuresService" ref="featuresService"/>

Review Comment:
   can be cleaned, removed.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+            installingFeatureStarted = true;
+            installFeatures();
+            checkStartupComplete();
+            return;
+        }

Review Comment:
   Should this be checked by the schedule task instead ?



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -142,25 +172,38 @@ public void checkExistingBundles() {
         checkStartupComplete();
     }
 
+    private void checkInBundlesList(Bundle bundle, Map<String, Boolean> bundles) {
+        if (bundle.getSymbolicName().startsWith("org.apache.unomi") && bundles.containsKey(bundle.getSymbolicName())) {
+            if (bundle.getState() == Bundle.ACTIVE) {
+                bundles.put(bundle.getSymbolicName(), true);
+            } else {
+                bundles.put(bundle.getSymbolicName(), false);
+            }
+        }
+    }
+
+    private void managedBundleEvent(Bundle bundle, Map<String, Boolean> bundles, Boolean start) {
+        if (bundle.getSymbolicName().startsWith("org.apache.unomi") && bundles.containsKey(bundle.getSymbolicName())) {
+            logger.info("Bundle {} was {}.", bundle.getSymbolicName(), start ? "started" : "stopped");
+            bundles.put(bundle.getSymbolicName(), start);
+            checkStartupComplete();

Review Comment:
   Careful here, it seem's this check on "checkStartupComplete" was only done in during bundle start, should we wrap it with if(start) ?



##########
tools/shell-commands/pom.xml:
##########
@@ -78,6 +79,12 @@
             <version>4.11</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.karaf.features</groupId>
+            <artifactId>org.apache.karaf.features.core</artifactId>
+            <version>${version.karaf}</version>
+            <scope>provided</scope>
+        </dependency>

Review Comment:
   Not sure it is still necessary here if removed from tools/shell-commands/src/main/java/org/apache/unomi/shell/services/internal/UnomiManagementServiceImpl.java



##########
tools/shell-commands/pom.xml:
##########
@@ -88,6 +95,11 @@
                 <configuration>
                     <instructions>
                         <Karaf-Commands>*</Karaf-Commands>
+                        <Import-Package>
+                            org.apache.karaf.util.tracker.annotation;resolution:=optional,
+                            org.eclipse.osgi.service.resolver;resolution:=optional,
+                            *
+                        </Import-Package>

Review Comment:
   Same could be cleaned.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+            installingFeatureStarted = true;
+            installFeatures();
+            checkStartupComplete();
+            return;
+        }
+        if (!allAdditionalBundleStarted()) {
+            startScheduler();
+            return;
+        }

Review Comment:
   This is a bit strange to have this check here since it is already done few lines before, could we update the first check instead of adding an other.
   I am a bit scared to introduce scenario with endless scheduled task or task being canceled without the installation really finished.



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {

Review Comment:
   We should add this new check inside the isStartupComplete directly



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jkevan commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jkevan commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r892034410


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcher.java:
##########
@@ -30,10 +30,12 @@ public interface BundleWatcher {
      * Retrieves the list of the server information objects, that include extensions. Each object includes the
      * name and version of the server, build time information and the event types
      * if recognizes as well as the capabilities supported by the system.
+     *
      * @return a list of ServerInfo objects with all the server information
      */
     List<ServerInfo> getServerInfos();
 
-
     boolean isStartupComplete();
+
+    boolean allAdditionalBundleStarted();

Review Comment:
   IS it necessary to have a public function about that ?
   For what purpose ? 
   If it's necessary, please add some JavaDoc. or remove it from Service interface and make it private.



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888761126


##########
tools/shell-commands/src/main/java/org/apache/unomi/shell/services/internal/UnomiManagementServiceImpl.java:
##########
@@ -31,13 +34,19 @@
  */
 public class UnomiManagementServiceImpl implements UnomiManagementService {
 
+    private static final Logger logger = LoggerFactory.getLogger(UnomiManagementServiceImpl.class);
+
     private BundleContext bundleContext;
+    private FeaturesService featuresService;
     private List<String> bundleSymbolicNames;
     private List<String> reversedBundleSymbolicNames;
 
+    private static final String CDP_GRAPHQL_FEATURE = "cdp-graphql-feature";

Review Comment:
   Removed



##########
tools/shell-commands/src/main/java/org/apache/unomi/shell/services/internal/UnomiManagementServiceImpl.java:
##########
@@ -31,13 +34,19 @@
  */
 public class UnomiManagementServiceImpl implements UnomiManagementService {
 
+    private static final Logger logger = LoggerFactory.getLogger(UnomiManagementServiceImpl.class);
+
     private BundleContext bundleContext;
+    private FeaturesService featuresService;

Review Comment:
   Removed



##########
tools/shell-commands/pom.xml:
##########
@@ -78,6 +79,12 @@
             <version>4.11</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.karaf.features</groupId>
+            <artifactId>org.apache.karaf.features.core</artifactId>
+            <version>${version.karaf}</version>
+            <scope>provided</scope>
+        </dependency>

Review Comment:
   Removed



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r892568679


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcher.java:
##########
@@ -30,10 +30,12 @@ public interface BundleWatcher {
      * Retrieves the list of the server information objects, that include extensions. Each object includes the
      * name and version of the server, build time information and the event types
      * if recognizes as well as the capabilities supported by the system.
+     *
      * @return a list of ServerInfo objects with all the server information
      */
     List<ServerInfo> getServerInfos();
 
-
     boolean isStartupComplete();
+
+    boolean allAdditionalBundleStarted();

Review Comment:
   It's used in the integrations test, as we need to wait that the additionalBundles are started and we need to wait that unomi is started before installing the additional bundles



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888761615


##########
tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml:
##########
@@ -59,13 +61,14 @@
                 <value>org.apache.unomi.router-rest</value>
                 <value>org.apache.unomi.shell-dev-commands</value>
                 <value>org.apache.unomi.web-tracker-wab</value>
-                <value>org.apache.unomi.cdp-graphql-api-impl</value>
                 <value>org.apache.unomi.groovy-actions-services</value>
                 <value>org.apache.unomi.groovy-actions-rest</value>
             </list>
         </property>
         <property name="bundleContext" ref="blueprintBundleContext"/>
+        <property name="featuresService" ref="featuresService"/>

Review Comment:
   Removed



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888779181


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+            installingFeatureStarted = true;
+            installFeatures();
+            checkStartupComplete();
+            return;
+        }
+        if (!allAdditionalBundleStarted()) {
+            startScheduler();
+            return;
+        }

Review Comment:
   The problem is that I need to wait that Unomi is fully started, otherwise I have a strange behaviour where I try to install features during the starting. Installing the bundles of a feature during this time, stop the execution and Unomi never start.



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888779445


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {
+            installingFeatureStarted = true;
+            installFeatures();
+            checkStartupComplete();
+            return;
+        }

Review Comment:
   Done



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888780615


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }

Review Comment:
   I can't because I have to wait that Unomi is started



##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }

Review Comment:
   I can't because I have to wait that Unomi is started before installing the features



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888761364


##########
tools/shell-commands/pom.xml:
##########
@@ -88,6 +95,11 @@
                 <configuration>
                     <instructions>
                         <Karaf-Commands>*</Karaf-Commands>
+                        <Import-Package>
+                            org.apache.karaf.util.tracker.annotation;resolution:=optional,
+                            org.eclipse.osgi.service.resolver;resolution:=optional,
+                            *
+                        </Import-Package>

Review Comment:
   Removed



##########
tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml:
##########
@@ -20,12 +20,14 @@
            xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
            xsi:schemaLocation="http://www.osgi.org/xmlns/blueprint/v1.0.0 http://www.osgi.org/xmlns/blueprint/v1.0.0/blueprint.xsd">
 
+    <reference id="featuresService" interface="org.apache.karaf.features.FeaturesService"/>
+

Review Comment:
   Removed



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r888780185


##########
lifecycle-watcher/src/main/java/org/apache/unomi/lifecycle/BundleWatcherImpl.java:
##########
@@ -223,50 +266,93 @@ private void displayLogsForInactiveServices() {
         });
     }
 
+    private void fillFeaturesToInstall() {
+        String installGraphQLFeature = bundleContext.getProperty("org.apache.unomi.graphql.feature.activated");
+        boolean graphQLToInstall = StringUtils.isNotBlank(installGraphQLFeature) && installGraphQLFeature.equals("true");
+        if (graphQLToInstall) {
+            featuresToInstall.add(CDP_GRAPHQL_FEATURE);
+            requiredBundlesFromFeatures.put("org.apache.unomi.cdp-graphql-api-impl", false);
+            requiredBundlesFromFeatures.put("org.apache.unomi.graphql-playground", false);
+        }
+    }
+
+    public boolean shouldInstallAdditionalFeatures() {
+        return !featuresToInstall.isEmpty();
+    }
+
+    private void installFeatures() {
+        List<String> installedFeatures = new ArrayList<>();
+        featuresToInstall.forEach(value -> {
+            try {
+                long featureStartupTime = System.currentTimeMillis();
+                if (!featuresService.isInstalled(featuresService.getFeature(value))) {
+                    System.out.println("Installing feature " + value);
+                    featuresService.installFeature(value, EnumSet.of(FeaturesService.Option.NoAutoRefreshManagedBundles,
+                            FeaturesService.Option.NoAutoRefreshUnmanagedBundles, FeaturesService.Option.NoAutoRefreshBundles));
+                    logger.info("Feature {} successfully installed in {} ms", value, System.currentTimeMillis() - featureStartupTime);
+                }
+                installedFeatures.add(value);
+            } catch (Exception e) {
+                logger.error("Error when installing {} feature", value, e);
+            }
+        });
+        installedFeatures.forEach(value -> featuresToInstall.remove(value));
+    }
+
+    private void startScheduler() {
+        if (scheduledFuture == null || scheduledFuture.isCancelled()) {
+            TimerTask task = new TimerTask() {
+                @Override
+                public void run() {
+                    displayLogsForInactiveBundles(requiredBundles);
+                    displayLogsForInactiveBundles(requiredBundlesFromFeatures);
+                    displayLogsForInactiveServices();
+                    checkStartupComplete();
+                }
+            };
+            scheduledFuture = scheduler
+                    .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
+        }
+    }
+
     private void checkStartupComplete() {
         if (!isStartupComplete()) {
-            if (scheduledFuture == null || scheduledFuture.isCancelled()) {
-                TimerTask task = new TimerTask() {
-                    @Override
-                    public void run() {
-                        displayLogsForInactiveBundles();
-                        displayLogsForInactiveServices();
-                        checkStartupComplete();
-                    }
-                };
-                scheduledFuture = scheduler
-                        .scheduleWithFixedDelay(task, checkStartupStateRefreshInterval, checkStartupStateRefreshInterval, TimeUnit.SECONDS);
-            }
+            startScheduler();
             return;
         }
         if (scheduledFuture != null) {
             scheduledFuture.cancel(true);
             scheduledFuture = null;
         }
+        if (shouldInstallAdditionalFeatures() && !installingFeatureStarted) {

Review Comment:
   I can't because I have to wait that Unomi is started



-- 
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@unomi.apache.org

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


[GitHub] [unomi] jsinovassin commented on a diff in pull request #432: UNOMI-582 GraphQL as feature flag

Posted by GitBox <gi...@apache.org>.
jsinovassin commented on code in PR #432:
URL: https://github.com/apache/unomi/pull/432#discussion_r892566744


##########
package/pom.xml:
##########
@@ -377,9 +377,8 @@
                         <feature>unomi-router-karaf-feature</feature>
                         <feature>unomi-web-tracker-karaf-kar</feature>
                         <feature>unomi-groovy-actions</feature>
-                        <feature>cdp-graphql-feature</feature>
                         <feature>unomi-rest-ui</feature>
-                        <feature>unomi-graphql-ui</feature>
+                        <feature>cdp-graphql-feature-denpendencies</feature>

Review Comment:
   I added it in the first try because there is some bundles in the cdp-graphql-feature-dependencies which are used by some other bundles (persistence for example). So the bundles was reloading and the unomi was not starting correctly, but with the last modifications, it's not necessary anymore. I'll do the changes



-- 
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@unomi.apache.org

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