andrii0lomakin commented on code in PR #3287:
URL: https://github.com/apache/tinkerpop/pull/3287#discussion_r2690019101


##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java:
##########
@@ -340,13 +336,270 @@ public long getEvaluationTimeout() {
 
     /**
      * Read configuration from a file into a new {@link Settings} object.
+     * <p>
+     * This method supports recursive includes of other YAML files over the 
"includes" property that contains
+     * a list of strings that can be:
+     * <ol>
+     *     <li>Relative paths to other YAML files</li>
+     *     <li>Absolute paths to other YAML files</li>
+     *     <li>Classpath resources</li>
+     * </ol>
+     * <p>
+     * If properties names of the included files or root file (file that 
contains "includes" property) are the same,
+     * they are overwritten forming a single property set. In any other cases 
just appended to the root file.
+     * "includes" can be nested and included files can also contain "includes" 
property.
+     * Included files can override properties of other included files, the 
root file in turn overriding properties of included files.
+     * <p>
+     * This is quite permissive strategy that works because we then map 
resulting YAML to <code>Settings</code> object
+     * preventing any configuration inconsistencies.
+     * <p>
+     * Abstract example:
+     * <code>base.yaml</code>
+     * <pre>
+     *   server:
+     *     connector: { port: 8080, protocol: 'http' }
+     *     logging: { level: 'INFO' }
+     * </pre>
+     * <code>root.yaml</code>
+     * <pre>
+     *   includes: ['base.yaml']
+     *   server:
+     *     connector: { port: 9090 } # Overwrite the port only
+     *     logging: { file: '/var/log/app.log' } # Add the file, keep level
+     * </pre>
+     * <p>
+     * Resulting configuration:
+     * <pre>
+     *   server:
+     *     connector: { port: 9090, protocol: 'http' }
+     *     logging: { level: 'INFO', file: '/var/log/app.log' }
+     * </pre>
      *
      * @param file the location of a Gremlin Server YAML configuration file
      * @return a new {@link Optional} object wrapping the created {@link 
Settings}
      */
-    public static Settings read(final String file) throws Exception {
-        final InputStream input = new FileInputStream(new File(file));
-        return read(input);
+    public static Settings read(final String file) {
+        final NodeMapper constructor = createDefaultYamlConstructor();
+        final Yaml yaml = new Yaml();
+
+        HashSet<String> loadStack = new HashSet<>();
+
+        // Normalize the initial path
+        String normalizedPath = normalizeInitialPath(file);
+        Node finalNode = loadNodeRecursive(yaml, normalizedPath, loadStack);
+        if (finalNode == null) {
+            return new Settings();
+        }
+
+        finalNode.setTag(new Tag(Settings.class));
+        return (Settings) constructor.map(finalNode);
+    }
+
+
+    private static Node loadNodeRecursive(Yaml yaml, String currentPath, 
HashSet<String> loadStack) {
+        try {
+            if (loadStack.contains(currentPath)) {

Review Comment:
   No, it is not, please check 
org.apache.tinkerpop.gremlin.server.SettingsTest#testIncludeBranches. Pushed as 
counter-prove.



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