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


##########
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<>();

Review Comment:
   nit: we tend to be explicit about `final` declarations - would you mind 
doing a pass through on the changes here to add `final` where applicable?



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