Cole-Greer commented on code in PR #3384:
URL: https://github.com/apache/tinkerpop/pull/3384#discussion_r3095131945


##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java:
##########
@@ -186,20 +189,108 @@ public ServerGremlinExecutor(final Settings settings, 
final ExecutorService grem
                 .forEach(kv -> this.graphManager.putGraph(kv.getKey(), (Graph) 
kv.getValue()));
 
         // script engine init may have constructed the TraversalSource 
bindings - store them in Graphs object
-        
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
-                .filter(kv -> kv.getValue() instanceof TraversalSource)
-                .forEach(kv -> {
-                    logger.info("A {} is now bound to [{}] with {}", 
kv.getValue().getClass().getSimpleName(), kv.getKey(), kv.getValue());
-                    this.graphManager.putTraversalSource(kv.getKey(), 
(TraversalSource) kv.getValue());
-                });
+        final boolean hasScriptTraversalSources = 
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
+                .anyMatch(kv -> kv.getValue() instanceof TraversalSource);
+        if (hasScriptTraversalSources) {
+            logger.warn("TraversalSource instances were created via script 
engine initialization scripts. " +
+                        "This approach is deprecated - use the 
'traversalSources' YAML configuration instead.");
+            
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
+                    .filter(kv -> kv.getValue() instanceof TraversalSource)
+                    .forEach(kv -> {
+                        logger.info("A {} is now bound to [{}] with {}", 
kv.getValue().getClass().getSimpleName(), kv.getKey(), kv.getValue());
+                        this.graphManager.putTraversalSource(kv.getKey(), 
(TraversalSource) kv.getValue());
+                    });
+        }
 
         // determine if the initialization scripts introduced LifeCycleHook 
objects - if so we need to gather them
         // up for execution
-        hooks = 
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
+        final boolean hasScriptHooks = 
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
+                .anyMatch(kv -> kv.getValue() instanceof LifeCycleHook);
+        if (hasScriptHooks) {
+            logger.warn("LifeCycleHook instances were created via script 
engine initialization scripts. " +
+                        "This approach is deprecated - use the 
'lifecycleHooks' YAML configuration instead.");
+        }
+        final List<LifeCycleHook> scriptHooks = 
gremlinExecutor.getScriptEngineManager().getBindings().entrySet().stream()
                 .filter(kv -> kv.getValue() instanceof LifeCycleHook)
                 .map(kv -> (LifeCycleHook) kv.getValue())
                 .collect(Collectors.toList());
 
+        // process declarative traversalSources from YAML config - track which 
graphs have explicit entries

Review Comment:
   I don't intend for users to mix and match groovy init scripts with the 
declarative approach, but I hadn't intended to restrict it either. My 
preference would be for users to completely migrate to the new configs in one 
atomic step, however I don't see any reason to prevent users from say 
converting their traversalSource configuration to the declarative system, while 
retaining an old groovy LifeCycleHook for a while to do data loading.
   
   In the long-term, they will need to move everything to the new system, but 
it seems reasonable to allow a progressive migration.
   
   I will followup on the order of operations there to see if there are likely 
to be bad interactions between the 2 systems.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to