Copilot commented on code in PR #531: URL: https://github.com/apache/atlas/pull/531#discussion_r2820238022
########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + Review Comment: The AtlasJanusGraphDatabase is instantiated but the return value is not used. Looking at similar tools like RepairIndex, they directly call AtlasJanusGraphDatabase.getGraphInstance() without instantiating the database object first. This unnecessary instantiation could be removed as the static initialization happens inside getGraphInstance(). ```suggestion ``` ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { + safeRollback(graph); + throw t; + } finally { + try { + graph.close(); + } catch (Exception e) { + LOG.warn("Failed to close graph", e); + } + } + } + + private static Object eval(GremlinGroovyScriptEngine engine, Bindings bindings, String query) throws ScriptException { + LOG.info("Executing Gremlin ({} chars)", query != null ? query.length() : 0); + return engine.eval(query, bindings); + } + + private static void finishTx(JanusGraph graph, boolean commit) { + if (!graph.tx().isOpen()) { + return; + } + + if (commit) { + graph.tx().commit(); + } else { + graph.tx().rollback(); + } + } + + private static void safeRollback(JanusGraph graph) { + try { + if (graph != null && graph.tx().isOpen()) { + graph.tx().rollback(); + } + } catch (Exception ignored) { + } + } + + private static Options buildOptions() { + Options options = new Options(); + + options.addOption(Option.builder("q") + .longOpt("query") + .hasArg() + .argName("gremlin") + .desc("Gremlin-Groovy to evaluate. Example: g.V().limit(5).valueMap(true).toList()") + .build()); + + options.addOption(Option.builder("f") + .longOpt("file") + .hasArg() + .argName("path") + .desc("Read Gremlin-Groovy script from file") + .build()); + + options.addOption(Option.builder() + .longOpt("commit") + .desc("Commit the transaction after evaluation (default: rollback)") + .build()); + + options.addOption(Option.builder("h") + .longOpt("help") + .desc("Print help") + .build()); + + return options; + } + + private static CommandLine parseArgs(String[] args) throws ParseException { + return new DefaultParser().parse(buildOptions(), args); + } + + private static void printHelp() { + HelpFormatter formatter = new HelpFormatter(); + + String header = "\nEmbedded Gremlin CLI for Apache Atlas (JanusGraph).\n\n" + + "Requires: -Datlas.conf=/path/to/atlas/conf\n"; + String footer = "\nExamples:\n" + + " -q \"g.V().limit(5).valueMap(true).toList()\"\n" + + " -f /tmp/query.groovy\n"; + + formatter.printHelp("GremlinCli", header, buildOptions(), footer, true); + } + + private static String getQuery(CommandLine cmd) throws IOException { + String q = cmd.getOptionValue("q"); + String f = cmd.getOptionValue("f"); + + if ((q == null || q.trim().isEmpty()) && (f == null || f.trim().isEmpty())) { + throw new IllegalArgumentException("Missing query. Provide -q/--query or -f/--file. Use -h for help."); + } + + if (q != null && f != null) { + throw new IllegalArgumentException("Provide only one of -q/--query or -f/--file."); + } + + if (q != null) { + return q; + } + + return new String(Files.readAllBytes(Paths.get(f)), StandardCharsets.UTF_8); Review Comment: Reading entire file contents into memory with Files.readAllBytes could cause OutOfMemoryError for large Groovy script files. Consider adding a file size check or using a streaming approach. For example, check the file size before reading and reject files larger than a reasonable threshold (e.g., 10MB) to prevent potential DoS scenarios. ########## tools/atlas-gremlin-cli/pom.xml: ########## @@ -0,0 +1,55 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.atlas</groupId> + <artifactId>apache-atlas</artifactId> + <version>3.0.0-SNAPSHOT</version> + <relativePath>../../</relativePath> + </parent> + + <artifactId>atlas-gremlin-cli-tool</artifactId> + <packaging>jar</packaging> + <name>Apache Atlas Gremlin CLI Tool</name> + <description>Apache Atlas Gremlin CLI Tool (embedded JanusGraph)</description> + + <dependencies> + + <dependency> + <groupId>commons-cli</groupId> + <artifactId>commons-cli</artifactId> + <version>${commons-cli.version}</version> + <scope>provided</scope> + </dependency> + + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-graphdb-janus</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + <scope>provided</scope> Review Comment: The tool relies on ATLAS_CLASSPATH being set by the user to provide all dependencies (commons-cli, atlas-graphdb-janus, slf4j-api, and their transitive dependencies). This differs from other tools like notification-analyzer which bundle all dependencies in their distribution. If ATLAS_CLASSPATH is not properly set or is missing required JARs, the tool will fail with ClassNotFoundException at runtime. Consider documenting the exact JAR requirements in the README or following the notification-analyzer pattern of bundling dependencies in the distribution for better user experience. ```suggestion </dependency> <dependency> <groupId>org.apache.atlas</groupId> <artifactId>atlas-graphdb-janus</artifactId> <version>${project.version}</version> </dependency> <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-api</artifactId> ``` ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { + safeRollback(graph); + throw t; + } finally { + try { + graph.close(); + } catch (Exception e) { + LOG.warn("Failed to close graph", e); + } Review Comment: The main method lacks proper exit code handling. Similar Atlas tools like RepairIndex and BulkFetchAndUpdate use System.exit with explicit exit codes (EXIT_CODE_SUCCESS=0, EXIT_CODE_FAILED=1) to signal success or failure to the calling process. Without this, the CLI will always return exit code 0 even when exceptions occur, which prevents shell scripts from detecting failures. Consider adding exit codes and System.exit calls similar to other tools in the repository. ```suggestion private static final int EXIT_CODE_SUCCESS = 0; private static final int EXIT_CODE_FAILED = 1; public static void main(String[] args) throws Exception { int exitCode = EXIT_CODE_SUCCESS; JanusGraph graph = null; try { CommandLine cmd = parseArgs(args); if (cmd.hasOption("h")) { printHelp(); return; } final String query = getQuery(cmd); final boolean commit = cmd.hasOption("commit"); new AtlasJanusGraphDatabase(); graph = AtlasJanusGraphDatabase.getGraphInstance(); try { GraphTraversalSource g = graph.traversal(); GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); Bindings bindings = engine.createBindings(); bindings.put("graph", graph); bindings.put("g", g); bindings.put("__", __.class); bindings.put("P", P.class); Object result = eval(engine, bindings, query); if (result instanceof Traversal) { result = ((Traversal<?, ?>) result).toList(); } System.out.println(String.valueOf(result)); finishTx(graph, commit); } catch (ScriptException se) { safeRollback(graph); LOG.error("Error executing Gremlin script", se); exitCode = EXIT_CODE_FAILED; } catch (Throwable t) { safeRollback(graph); LOG.error("Unexpected error executing Gremlin CLI", t); exitCode = EXIT_CODE_FAILED; } } catch (Exception e) { LOG.error("Error running Gremlin CLI", e); exitCode = EXIT_CODE_FAILED; } finally { if (graph != null) { try { graph.close(); } catch (Exception e) { LOG.warn("Failed to close graph", e); } } System.exit(exitCode); ``` ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { Review Comment: Exception handling is inconsistent with other tools in the repository. RepairIndex and BulkFetchAndUpdate both use LOG.error() with the message and exception together in a single call. The catch blocks here rethrow exceptions without logging error messages, which makes debugging harder for users. Consider logging errors before rethrowing, similar to the pattern: LOG.error("Failed!", e). ```suggestion } catch (ScriptException se) { LOG.error("Gremlin script execution failed", se); safeRollback(graph); throw se; } catch (Throwable t) { LOG.error("Gremlin CLI execution failed", t); ``` ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { + safeRollback(graph); + throw t; + } finally { + try { + graph.close(); + } catch (Exception e) { + LOG.warn("Failed to close graph", e); + } + } + } + + private static Object eval(GremlinGroovyScriptEngine engine, Bindings bindings, String query) throws ScriptException { + LOG.info("Executing Gremlin ({} chars)", query != null ? query.length() : 0); + return engine.eval(query, bindings); + } + + private static void finishTx(JanusGraph graph, boolean commit) { + if (!graph.tx().isOpen()) { + return; + } + + if (commit) { + graph.tx().commit(); + } else { + graph.tx().rollback(); + } + } + + private static void safeRollback(JanusGraph graph) { + try { + if (graph != null && graph.tx().isOpen()) { + graph.tx().rollback(); + } + } catch (Exception ignored) { + } + } + + private static Options buildOptions() { + Options options = new Options(); + + options.addOption(Option.builder("q") + .longOpt("query") + .hasArg() + .argName("gremlin") + .desc("Gremlin-Groovy to evaluate. Example: g.V().limit(5).valueMap(true).toList()") + .build()); + + options.addOption(Option.builder("f") + .longOpt("file") + .hasArg() + .argName("path") + .desc("Read Gremlin-Groovy script from file") + .build()); + + options.addOption(Option.builder() + .longOpt("commit") + .desc("Commit the transaction after evaluation (default: rollback)") + .build()); + + options.addOption(Option.builder("h") + .longOpt("help") + .desc("Print help") + .build()); + + return options; + } + + private static CommandLine parseArgs(String[] args) throws ParseException { + return new DefaultParser().parse(buildOptions(), args); + } + + private static void printHelp() { + HelpFormatter formatter = new HelpFormatter(); + + String header = "\nEmbedded Gremlin CLI for Apache Atlas (JanusGraph).\n\n" + + "Requires: -Datlas.conf=/path/to/atlas/conf\n"; + String footer = "\nExamples:\n" + + " -q \"g.V().limit(5).valueMap(true).toList()\"\n" + + " -f /tmp/query.groovy\n"; + + formatter.printHelp("GremlinCli", header, buildOptions(), footer, true); + } + + private static String getQuery(CommandLine cmd) throws IOException { + String q = cmd.getOptionValue("q"); + String f = cmd.getOptionValue("f"); + + if ((q == null || q.trim().isEmpty()) && (f == null || f.trim().isEmpty())) { + throw new IllegalArgumentException("Missing query. Provide -q/--query or -f/--file. Use -h for help."); + } + + if (q != null && f != null) { + throw new IllegalArgumentException("Provide only one of -q/--query or -f/--file."); + } + + if (q != null) { + return q; + } + + return new String(Files.readAllBytes(Paths.get(f)), StandardCharsets.UTF_8); Review Comment: No validation is performed on the file path before reading. This could potentially allow reading arbitrary files that the Atlas process has access to. Consider validating that the file path doesn't contain directory traversal patterns (..) or restrict it to a specific directory. Additionally, verify that the file is readable and exists before attempting to read it to provide better error messages. ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { + safeRollback(graph); + throw t; + } finally { + try { + graph.close(); + } catch (Exception e) { + LOG.warn("Failed to close graph", e); + } + } Review Comment: The GraphTraversalSource 'g' should be closed when done, as it may hold resources. Consider adding g.close() in the finally block, similar to how graph.close() is handled. This ensures proper resource cleanup even when exceptions occur. ########## tools/atlas-gremlin-cli/src/main/java/org/apache/atlas/tools/GremlinCli.java: ########## @@ -0,0 +1,185 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.atlas.tools; + +import org.apache.atlas.repository.graphdb.janus.AtlasJanusGraphDatabase; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.janusgraph.core.JanusGraph; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.script.Bindings; +import javax.script.ScriptException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; + +public class GremlinCli { + private static final Logger LOG = LoggerFactory.getLogger(GremlinCli.class); + + public static void main(String[] args) throws Exception { + CommandLine cmd = parseArgs(args); + + if (cmd.hasOption("h")) { + printHelp(); + return; + } + + final String query = getQuery(cmd); + final boolean commit = cmd.hasOption("commit"); + + new AtlasJanusGraphDatabase(); + + JanusGraph graph = AtlasJanusGraphDatabase.getGraphInstance(); + + try { + GraphTraversalSource g = graph.traversal(); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + + Bindings bindings = engine.createBindings(); + bindings.put("graph", graph); + bindings.put("g", g); + bindings.put("__", __.class); + bindings.put("P", P.class); + + Object result = eval(engine, bindings, query); + + if (result instanceof Traversal) { + result = ((Traversal<?, ?>) result).toList(); + } + + System.out.println(String.valueOf(result)); + + finishTx(graph, commit); + } catch (ScriptException se) { + safeRollback(graph); + throw se; + } catch (Throwable t) { + safeRollback(graph); + throw t; + } finally { + try { + graph.close(); + } catch (Exception e) { + LOG.warn("Failed to close graph", e); + } + } Review Comment: The GremlinGroovyScriptEngine should be cleaned up after use by calling reset() to prevent memory leaks from script compilation. The existing codebase pattern in AtlasJanusGraph shows that the script engine should be released after execution. Consider adding a finally block to release the engine, similar to how it's done in AtlasJanusGraph.releaseGremlinScriptEngine(). ########## tools/atlas-gremlin-cli/src/main/resources/atlas-logback.xml: ########## @@ -0,0 +1,47 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<configuration> + <appender name="console" class="ch.qos.logback.core.ConsoleAppender"> + <param name="Target" value="System.out"/> + <encoder> + <pattern>%date [%thread] %level{5} [%file:%line] %msg%n</pattern> + </encoder> + <filter class="ch.qos.logback.classic.filter.ThresholdFilter"> + <level>INFO</level> + </filter> + </appender> Review Comment: The console appender is defined but never used in the root logger configuration. Line 44-45 only reference the FILE appender. If console output is intended for debugging purposes, it should be added to the root logger, or if it's not needed, the console appender definition can be removed to avoid confusion. -- 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]
