TINKERPOP-1534 Improve GraphProvider ability to release resources Specifically, made AbstractGremlinSuite attept to close() a GraphProvider if it implemented AutoCloseable. Added better logging to gremlin-python server start/stop script. Removed DriverRemoteConnectionTest as it was an ignored test anyway and a remnant of the original way we tested gremlin-python. Implemented AutoCloseable on RemoteGraphProvider to kill Gremlin Server which is no longer started statically.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/0f84e437 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/0f84e437 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/0f84e437 Branch: refs/heads/TINKERPOP-1534 Commit: 0f84e4378d82bcefb79071d8d14a92202a7a6445 Parents: 4ff93ab Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Oct 28 11:55:47 2016 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Wed Nov 2 11:41:35 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + gremlin-python/pom.xml | 15 +- .../driver/DriverRemoteConnectionTest.java | 153 ------------------- .../tinkerpop/gremlin/server/GremlinServer.java | 5 + .../driver/remote/RemoteGraphProvider.java | 20 ++- .../tinkerpop/gremlin/AbstractGremlinSuite.java | 18 +++ .../apache/tinkerpop/gremlin/GraphManager.java | 8 +- 7 files changed, 57 insertions(+), 163 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 882f6e9..faec861 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Improved ability to release resources in `GraphProvider` instances in the test suite. * Added a `force` option for killing sessions without waiting for transaction close or timeout of a currently running job or multiple jobs. * Deprecated `Session.kill()` and `Session.manualKill()`. * Added `choose(predicate,traversal)` and `choose(traversal,traversal)` to effect if/then-semantics (no else). Equivalent to `choose(x,y,identity())`. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-python/pom.xml ---------------------------------------------------------------------- diff --git a/gremlin-python/pom.xml b/gremlin-python/pom.xml index 1a550fa..923380d 100644 --- a/gremlin-python/pom.xml +++ b/gremlin-python/pom.xml @@ -526,10 +526,17 @@ import org.apache.tinkerpop.gremlin.server.GremlinServer if (${skipTests}) return -log.info("Tests for native gremlin-python complete - shutting down Gremlin Server") -project.getContextValue("gremlin.py.server").stop().join() -project.getContextValue("gremlin.py.server.secure").stop().join() -log.info("Gremlin Server shutdown") +log.info("Tests for native gremlin-python complete") + +def server = project.getContextValue("gremlin.py.server") +log.info("Shutting down $server") +server.stop().join() + +def serverSecure = project.getContextValue("gremlin.py.server.secure") +log.info("Shutting down $serverSecure") +serverSecure.stop().join() + +log.info("All Gremlin Server instances are shutdown for gremlin-python") ]]> </script> </scripts> http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/driver/DriverRemoteConnectionTest.java ---------------------------------------------------------------------- diff --git a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/driver/DriverRemoteConnectionTest.java b/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/driver/DriverRemoteConnectionTest.java deleted file mode 100644 index 27952a8..0000000 --- a/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/driver/DriverRemoteConnectionTest.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * 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.tinkerpop.gremlin.python.driver; - -import org.apache.tinkerpop.gremlin.TestHelper; -import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalSideEffects; -import org.apache.tinkerpop.gremlin.server.GremlinServer; -import org.apache.tinkerpop.gremlin.server.Settings; -import org.junit.BeforeClass; -import org.junit.Test; - -import java.io.BufferedReader; -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.Writer; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -/** - * @author Marko A. Rodriguez (http://markorodriguez.com) - */ -@org.junit.Ignore -public class DriverRemoteConnectionTest { - - private static boolean PYTHON_EXISTS = false; - - @BeforeClass - public static void setup() { - try { - final Optional<String> pythonVersion = new BufferedReader(new InputStreamReader(Runtime.getRuntime().exec("python --version").getErrorStream())) - .lines() - .filter(line -> line.trim().startsWith("Python ")) - .findAny(); - PYTHON_EXISTS = pythonVersion.isPresent(); - System.out.println("Python virtual machine: " + pythonVersion.orElse("None")); - if (PYTHON_EXISTS) - new GremlinServer(Settings.read(DriverRemoteConnectionTest.class.getResourceAsStream("gremlin-server-modern-secure-py.yaml"))).start().join(); - } catch (final Exception ex) { - ex.printStackTrace(); - } - } - - private static List<String> submit(final String... scriptLines) throws IOException { - final StringBuilder builder = new StringBuilder(); - builder.append("from gremlin_python import statics\n"); - builder.append("from gremlin_python.structure.graph import Graph\n"); - builder.append("from gremlin_python.driver.driver_remote_connection import DriverRemoteConnection\n"); - builder.append("from gremlin_python.structure.io.graphson import GraphSONWriter\n\n"); - builder.append("statics.load_statics(globals())\n"); - builder.append("graph = Graph()\n"); - builder.append("g = graph.traversal().withRemote(DriverRemoteConnection('ws://localhost:8182','g',username='stephen', password='password'))\n"); - for (int i = 0; i < scriptLines.length - 1; i++) { - builder.append(scriptLines[i] + "\n"); - } - builder.append("final = " + scriptLines[scriptLines.length - 1] + "\n"); - builder.append("if isinstance(final,dict):\n"); - builder.append(" for key in final.keys():\n"); - builder.append(" print (str(key),str(final[key]))\n"); - builder.append("elif isinstance(final,str):\n"); - builder.append(" print final\n"); - builder.append("else:\n"); - builder.append(" for result in final:\n"); - builder.append(" print result\n\n"); - - File file = TestHelper.generateTempFile(DriverRemoteConnectionTest.class, "temp", "py"); - final Writer writer = new BufferedWriter(new FileWriter(file.getAbsoluteFile())); - writer.write(builder.toString()); - writer.flush(); - writer.close(); - - final BufferedReader reader = new BufferedReader(new InputStreamReader(Runtime.getRuntime().exec("python " + file.getAbsolutePath()).getInputStream())); - final List<String> lines = reader.lines().map(String::trim).collect(Collectors.toList()); - reader.close(); - file.delete(); - return lines; - - } - - @Test - public void testTraversals() throws Exception { - if (!PYTHON_EXISTS) return; - - List<String> result = DriverRemoteConnectionTest.submit("g.V().count()"); - assertEquals(1, result.size()); - assertEquals("6", result.get(0)); - // - result = DriverRemoteConnectionTest.submit("g.V(1).out('created').name"); - assertEquals(1, result.size()); - assertEquals("lop", result.get(0)); - // - result = DriverRemoteConnectionTest.submit("g.V(1).out()"); - assertEquals(3, result.size()); - assertTrue(result.contains("v[4]")); - assertTrue(result.contains("v[2]")); - assertTrue(result.contains("v[3]")); - // - result = DriverRemoteConnectionTest.submit("g.V().repeat(out()).times(2).name"); - assertEquals(2, result.size()); - assertTrue(result.contains("lop")); - assertTrue(result.contains("ripple")); - } - - @Test - public void testSideEffects() throws Exception { - if (!PYTHON_EXISTS) return; - - List<String> result = DriverRemoteConnectionTest.submit( - "t = g.V().out().iterate()", - "str(t.side_effects)"); - assertEquals(1, result.size()); - assertEquals(new DefaultTraversalSideEffects().toString(), result.get(0)); - // - result = DriverRemoteConnectionTest.submit( - "t = g.V().out('created').groupCount('m').by('name').iterate()", - "t.side_effects['m']"); - assertEquals(2, result.size()); - assertTrue(result.contains("('ripple', '1')")); - assertTrue(result.contains("('lop', '3')")); - // - result = DriverRemoteConnectionTest.submit( - "t = g.V().out('created').groupCount('m').by('name').aggregate('n').iterate()", - "t.side_effects.keys()"); - assertEquals(2, result.size()); - assertTrue(result.contains("m")); - assertTrue(result.contains("n")); - } - -} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java index 268a903..ed0fd7c 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java @@ -405,4 +405,9 @@ public class GremlinServer { if (settings.gremlinPool == 0) settings.gremlinPool = Runtime.getRuntime().availableProcessors(); } + + @Override + public String toString() { + return "GremlinServer " + settings.host + ":" + settings.port; + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java index eae229a..f071671 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java @@ -36,12 +36,13 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; /** * @author Stephen Mallette (http://stephen.genoprime.com) */ -public class RemoteGraphProvider extends AbstractGraphProvider { +public class RemoteGraphProvider extends AbstractGraphProvider implements AutoCloseable { private static final Set<Class> IMPLEMENTATION = new HashSet<Class>() {{ add(RemoteGraph.class); }}; @@ -52,11 +53,20 @@ public class RemoteGraphProvider extends AbstractGraphProvider { //private final Cluster cluster = Cluster.build().maxContentLength(1024000).serializer(Serializers.GRAPHSON_V2D0).create(); private final Client client = cluster.connect(); - static { + public RemoteGraphProvider() { try { startServer(); } catch (Exception ex) { - ex.printStackTrace(); + throw new RuntimeException(ex); + } + } + + @Override + public void close() throws Exception { + try { + stopServer(); + } catch (Exception ex) { + throw new RuntimeException(ex); } } @@ -120,11 +130,11 @@ public class RemoteGraphProvider extends AbstractGraphProvider { server = new GremlinServer(settings); - server.start().join(); + server.start().get(100, TimeUnit.SECONDS); } public static void stopServer() throws Exception { - server.stop().join(); + server.stop().get(100, TimeUnit.SECONDS); server = null; } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/AbstractGremlinSuite.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/AbstractGremlinSuite.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/AbstractGremlinSuite.java index fa9d0e6..b896f8b 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/AbstractGremlinSuite.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/AbstractGremlinSuite.java @@ -29,6 +29,7 @@ import org.junit.runner.notification.RunNotifier; import org.junit.runners.Suite; import org.junit.runners.model.InitializationError; import org.junit.runners.model.RunnerBuilder; +import org.junit.runners.model.Statement; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -213,6 +214,23 @@ public abstract class AbstractGremlinSuite extends Suite { afterTestExecution((Class<? extends AbstractGremlinTest>) runner.getDescription().getTestClass()); } + @Override + protected Statement withAfterClasses(final Statement statement) { + final Statement wrappedStatement = new Statement() { + @Override + public void evaluate() throws Throwable { + statement.evaluate(); + + // release resources in GraphProviders that implement AutoCloseable + final GraphProvider gp = GraphManager.getGraphProvider(); + if (gp instanceof AutoCloseable) + ((AutoCloseable) gp).close(); + } + }; + + return super.withAfterClasses(wrappedStatement); + } + /** * Called just prior to test class execution. Return false to ignore test class. By default this always returns * true. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0f84e437/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/GraphManager.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/GraphManager.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/GraphManager.java index 571658d..6886465 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/GraphManager.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/GraphManager.java @@ -72,7 +72,7 @@ public class GraphManager { * When {@link #openTestGraph(Configuration)} is called the created object is stored in a list and when tests are * complete the {@link #tryClearGraphs()} is called. When this is called, an attempt is made to close all open graphs. */ - public static class ManagedGraphProvider implements GraphProvider { + public static class ManagedGraphProvider implements GraphProvider, AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(ManagedGraphProvider.class); private final GraphProvider innerGraphProvider; private final List<Pair<Graph, Configuration>> openGraphs = new ArrayList<>(); @@ -177,6 +177,12 @@ public class GraphManager { public Optional<TestListener> getTestListener() { return innerGraphProvider.getTestListener(); } + + @Override + public void close() throws Exception { + if (innerGraphProvider instanceof AutoCloseable) + ((AutoCloseable) innerGraphProvider).close(); + } } }