You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemds.apache.org by ba...@apache.org on 2020/09/21 09:22:17 UTC
[systemds] branch master updated: [MINOR] Python API hardening,
and stability
This is an automated email from the ASF dual-hosted git repository.
baunsgaard pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/master by this push:
new 1f136eb [MINOR] Python API hardening, and stability
1f136eb is described below
commit 1f136eb3d0fdc17cc66c5c93006cc3ab6d81fc34
Author: baunsgaard <ba...@tugraz.at>
AuthorDate: Sun Sep 6 12:30:31 2020 +0200
[MINOR] Python API hardening, and stability
Minor changes in Python API startup for ease of startup if SystemDS is
installed somewhere else it will use that SystemDS.
This practically means that if you have SystemDS home set, it will allow
the python to use that SystemDS, while if it is not set, it will default
back to the installed jar files from the PIP install.
This is a debated topic in #992, where it is argued that it would make
it harder for a user if the PIP does not contain the jar files.
- Fix dual setup of systemds_context
- Added to usertest
---
.../java/org/apache/sysds/api/PythonDMLScript.java | 8 ++-
.../python/systemds/context/systemds_context.py | 50 +++++++++++++------
.../java/org/apache/sysds/test/usertest/Base.java | 5 +-
.../sysds/test/usertest/UserInterfaceTest.java | 2 -
.../sysds/test/usertest/pythonapi/StartupTest.java | 58 ++++++++++++++++++++++
5 files changed, 104 insertions(+), 19 deletions(-)
diff --git a/src/main/java/org/apache/sysds/api/PythonDMLScript.java b/src/main/java/org/apache/sysds/api/PythonDMLScript.java
index 62ae738..e7251e7 100644
--- a/src/main/java/org/apache/sysds/api/PythonDMLScript.java
+++ b/src/main/java/org/apache/sysds/api/PythonDMLScript.java
@@ -40,7 +40,13 @@ public class PythonDMLScript {
* @param args Command line arguments.
*/
public static void main(String[] args) {
- start(Integer.parseInt(args[0]));
+ if(args.length != 1) {
+ throw new IllegalArgumentException("Python DML Script should be initialized with a singe number argument");
+ }
+ else {
+ int port = Integer.parseInt(args[0]);
+ start(port);
+ }
}
private static void start(int port) {
diff --git a/src/main/python/systemds/context/systemds_context.py b/src/main/python/systemds/context/systemds_context.py
index 16e99c8..7c32e75 100644
--- a/src/main/python/systemds/context/systemds_context.py
+++ b/src/main/python/systemds/context/systemds_context.py
@@ -54,34 +54,34 @@ class SystemDSContext(object):
that can be read from to get the printed statements from the JVM.
"""
- systemds_java_path = os.path.join(get_module_dir(), "systemds-java")
+ root = os.environ.get("SYSTEMDS_ROOT")
+ if root == None:
+ # If there is no systemds install default to use the PIP packaged java files.
+ root = os.path.join(get_module_dir(), "systemds-java")
+
# nt means its Windows
cp_separator = ";" if os.name == "nt" else ":"
- lib_cp = os.path.join(systemds_java_path, "lib", "*")
- systemds_cp = os.path.join(systemds_java_path, "*")
- classpath = cp_separator.join([lib_cp, systemds_cp])
+ lib_cp = os.path.join(root, "target","lib", "*")
+ systemds_cp = os.path.join(root,"target","SystemDS.jar")
+ classpath = cp_separator.join([lib_cp , systemds_cp])
- # TODO make use of JavaHome env-variable if set to find java, instead of just using any java available.
command = ["java", "-cp", classpath]
- sys_root = os.environ.get("SYSTEMDS_ROOT")
- if sys_root != None:
- files = glob(os.path.join(sys_root, "conf", "log4j*.properties"))
+ if os.environ.get("SYSTEMDS_ROOT") != None:
+ files = glob(os.path.join(root, "conf", "log4j*.properties"))
if len(files) > 1:
- print("WARNING: Multiple logging files")
+ print("WARNING: Multiple logging files found selecting: " + files[0])
if len(files) == 0:
print("WARNING: No log4j file found at: "
- + os.path.join(sys_root, "conf")
+ + os.path.join(root, "conf")
+ " therefore using default settings")
else:
- # print("Using Log4J file at " + files[0])
command.append("-Dlog4j.configuration=file:" + files[0])
- else:
- print("Default Log4J used, since environment $SYSTEMDS_ROOT not set")
command.append("org.apache.sysds.api.PythonDMLScript")
# TODO add an argument parser here
+
# Find a random port, and hope that no other process
# steals it while we wait for the JVM to startup
port = self.__get_open_port()
@@ -89,7 +89,27 @@ class SystemDSContext(object):
process = Popen(command, stdout=PIPE, stdin=PIPE, stderr=PIPE)
first_stdout = process.stdout.readline()
- assert (b"Server Started" in first_stdout), "Error JMLC Server not Started"
+
+ if(b"GatewayServer Started" in first_stdout):
+ print("Startup success")
+ else:
+ stderr = process.stderr.readline().decode("utf-8")
+ if(len(stderr) > 1):
+ raise Exception("Exception in startup of GatewayServer: " + stderr)
+ outputs = []
+ outputs.append(first_stdout.decode("utf-8"))
+ max_tries = 10
+ for i in range(max_tries):
+ next_line = process.stdout.readline()
+ if(b"GatewayServer Started" in next_line):
+ print("WARNING: Stdout corrupted by prints: " + str(outputs))
+ print("Startup success")
+ break
+ else:
+ outputs.append(next_line)
+
+ if (i == max_tries-1):
+ raise Exception("Error in startup of systemDS gateway process: \n gateway StdOut: " + str(outputs) + " \n gateway StdErr" + process.stderr.readline().decode("utf-8") )
# Handle Std out from the subprocess.
self.__stdout = Queue()
@@ -153,7 +173,7 @@ class SystemDSContext(object):
def __get_open_port(self):
"""Get a random available port."""
# TODO Verify that it is not taking some critical ports change to select a good port range.
- # TODO If it tries to select a port already in use, find anotherone. (recursion)
+ # TODO If it tries to select a port already in use, find another.
# https://stackoverflow.com/questions/2838244/get-open-tcp-port-in-python
import socket
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
diff --git a/src/test/java/org/apache/sysds/test/usertest/Base.java b/src/test/java/org/apache/sysds/test/usertest/Base.java
index 52e284d..cf19a84 100644
--- a/src/test/java/org/apache/sysds/test/usertest/Base.java
+++ b/src/test/java/org/apache/sysds/test/usertest/Base.java
@@ -78,9 +78,12 @@ public class Base {
public static Pair<String, String> runThread(String script) {
String fullDMLScriptName = BASE_FOLDER + script;
+ return runThread(new String[]{"-f", fullDMLScriptName});
+ }
+ public static Pair<String, String> runThread(String[] args) {
Thread t = new Thread(() -> {
- DMLScript.main(new String[] {"-f", fullDMLScriptName});
+ DMLScript.main(args);
});
ByteArrayOutputStream buff = new ByteArrayOutputStream();
diff --git a/src/test/java/org/apache/sysds/test/usertest/UserInterfaceTest.java b/src/test/java/org/apache/sysds/test/usertest/UserInterfaceTest.java
index 67ebca2..04db802 100644
--- a/src/test/java/org/apache/sysds/test/usertest/UserInterfaceTest.java
+++ b/src/test/java/org/apache/sysds/test/usertest/UserInterfaceTest.java
@@ -46,8 +46,6 @@ public class UserInterfaceTest extends Base {
public void SyntaxError(){
Pair<String,String> res = runThread("SyntaxError.dml");
assertEquals("",res.getRight());
- System.out.println(res.getLeft());
- System.out.println(res.getRight());
assertTrue(res.getLeft().contains("An Error Occured :"));
assertTrue(res.getLeft().contains("[Syntax error]"));
assertTrue(res.getLeft().contains("ParseException --"));
diff --git a/src/test/java/org/apache/sysds/test/usertest/pythonapi/StartupTest.java b/src/test/java/org/apache/sysds/test/usertest/pythonapi/StartupTest.java
new file mode 100644
index 0000000..561bdf1
--- /dev/null
+++ b/src/test/java/org/apache/sysds/test/usertest/pythonapi/StartupTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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.sysds.test.usertest.pythonapi;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+
+import org.apache.sysds.api.PythonDMLScript;
+import org.junit.Test;
+
+/** Simple tests to verify startup of Python Gateway server happens without crashes */
+public class StartupTest {
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testStartupIncorrect_1() {
+ PythonDMLScript.main(new String[] {});
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testStartupIncorrect_2() {
+ PythonDMLScript.main(new String[] {""});
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testStartupIncorrect_3() {
+ PythonDMLScript.main(new String[] {"131", "131"});
+ }
+
+ @Test(expected = NumberFormatException.class)
+ public void testStartupIncorrect_4() {
+ PythonDMLScript.main(new String[] {"Hello"});
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testStartupIncorrect_5() {
+ // Number out of range
+ PythonDMLScript.main(new String[] {"918757"});
+ }
+}