You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by js...@apache.org on 2018/08/22 02:48:40 UTC
incubator-livy git commit: [LIVY-498][REPL] Fix Windows CRLF line
ending issue in SparkR interpreter
Repository: incubator-livy
Updated Branches:
refs/heads/master 551dd5c27 -> 8027ca708
[LIVY-498][REPL] Fix Windows CRLF line ending issue in SparkR interpreter
## What changes were proposed in this pull request?
If the issued query contains CRLF EOL, it will be failed to execute on *nix machine. This happens when submitting queries from Windows machine and executing on Linux machine.
So here propose to convert statement to match system's EOL.
## How was this patch tested?
New UT added.
Author: jerryshao <ss...@hortonworks.com>
Closes #105 from jerryshao/LIVY-498.
Project: http://git-wip-us.apache.org/repos/asf/incubator-livy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-livy/commit/8027ca70
Tree: http://git-wip-us.apache.org/repos/asf/incubator-livy/tree/8027ca70
Diff: http://git-wip-us.apache.org/repos/asf/incubator-livy/diff/8027ca70
Branch: refs/heads/master
Commit: 8027ca708fdc3df9a5b08d2d33d0436018154bcc
Parents: 551dd5c
Author: jerryshao <ss...@hortonworks.com>
Authored: Wed Aug 22 10:48:33 2018 +0800
Committer: jerryshao <ss...@hortonworks.com>
Committed: Wed Aug 22 10:48:33 2018 +0800
----------------------------------------------------------------------
.../main/scala/org/apache/livy/EOLUtils.scala | 101 +++++++++++++++++++
.../scala/org/apache/livy/EOLUtilsSuite.scala | 57 +++++++++++
.../org/apache/livy/test/InteractiveIT.scala | 2 +
.../scala/org/apache/livy/repl/ReplDriver.scala | 6 +-
4 files changed, 163 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/8027ca70/core/src/main/scala/org/apache/livy/EOLUtils.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/livy/EOLUtils.scala b/core/src/main/scala/org/apache/livy/EOLUtils.scala
new file mode 100644
index 0000000..10f2c62
--- /dev/null
+++ b/core/src/main/scala/org/apache/livy/EOLUtils.scala
@@ -0,0 +1,101 @@
+/*
+ * 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.livy
+
+/**
+ * Helper class to deal with end-of-line markers in text files.
+ */
+object EOLUtils {
+ /** Unix-style end-of-line marker (LF) */
+ private val EOL_UNIX: String = "\n"
+
+ /** Windows-style end-of-line marker (CRLF) */
+ private val EOL_WINDOWS: String = "\r\n"
+
+ /** "Old Mac"-style end-of-line marker (CR) */
+ private val EOL_OLD_MAC: String = "\r"
+
+ /** Default end-of-line marker on current syste */
+ private val EOL_SYSTEM_DEFAULT: String = System.getProperty("line.separator")
+
+ object Mode extends Enumeration {
+ type Mode = Value
+
+ val LF, CRLF, CR = Value
+
+ lazy val SYSTEM_DEFAULT: Mode = {
+ val tmp = if (EOL_SYSTEM_DEFAULT == EOL_UNIX) {
+ LF
+ } else if (EOL_SYSTEM_DEFAULT == EOL_WINDOWS) {
+ CRLF
+ } else if (EOL_SYSTEM_DEFAULT == EOL_OLD_MAC) {
+ CR
+ } else {
+ null
+ }
+
+ if (tmp == null) {
+ throw new IllegalStateException("Could not determine system default end-of-line marker")
+ }
+ tmp
+ }
+
+ private def determineEOL(s: String): Mode = {
+ val charArray = s.toCharArray
+
+ var prev: Char = null.asInstanceOf[Char]
+ for (ch <- charArray) {
+ if (ch == '\n') {
+ if (prev == '\r') {
+ return CRLF
+ } else {
+ return LF
+ }
+ } else if (prev == '\r') {
+ return CR
+ }
+
+ prev = ch
+ }
+
+ null
+ }
+
+ def hasWindowsEOL(s: String): Boolean = determineEOL(s) == CRLF
+
+ def hasUnixEOL(s: String): Boolean = determineEOL(s) == LF
+
+ def hasOldMacEOL(s: String): Boolean = determineEOL(s) == CR
+
+ def hasSystemDefaultEOL(s: String): Boolean = determineEOL(s) == SYSTEM_DEFAULT
+ }
+
+ def convertToSystemEOL(s: String): String = convertLineEndings(s, EOL_SYSTEM_DEFAULT)
+
+ private def convertLineEndings(s: String, eol: String): String = {
+ if (Mode.hasWindowsEOL(s)) {
+ s.replaceAll(EOL_WINDOWS, eol)
+ } else if (Mode.hasUnixEOL(s)) {
+ s.replaceAll(EOL_UNIX, eol)
+ } else if (Mode.hasOldMacEOL(s)) {
+ s.replaceAll(EOL_OLD_MAC, eol)
+ } else {
+ s
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/8027ca70/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala
new file mode 100644
index 0000000..8ee73a1
--- /dev/null
+++ b/core/src/test/scala/org/apache/livy/EOLUtilsSuite.scala
@@ -0,0 +1,57 @@
+/*
+ * 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.livy
+
+import org.scalatest.FunSuite
+
+class EOLUtilsSuite extends FunSuite with LivyBaseUnitTestSuite {
+
+ test("check EOL") {
+ val s1 = "test\r\ntest"
+ assert(!EOLUtils.Mode.hasUnixEOL(s1))
+ assert(!EOLUtils.Mode.hasOldMacEOL(s1))
+ assert(EOLUtils.Mode.hasWindowsEOL(s1))
+
+ val s2 = "test\ntest"
+ assert(EOLUtils.Mode.hasUnixEOL(s2))
+ assert(!EOLUtils.Mode.hasOldMacEOL(s2))
+ assert(!EOLUtils.Mode.hasWindowsEOL(s2))
+
+ val s3 = "test\rtest"
+ assert(!EOLUtils.Mode.hasUnixEOL(s3))
+ assert(EOLUtils.Mode.hasOldMacEOL(s3))
+ assert(!EOLUtils.Mode.hasWindowsEOL(s3))
+
+ val s4 = "testtest"
+ assert(!EOLUtils.Mode.hasUnixEOL(s4))
+ assert(!EOLUtils.Mode.hasOldMacEOL(s4))
+ assert(!EOLUtils.Mode.hasWindowsEOL(s4))
+ }
+
+ test("convert EOL") {
+ val s1 = "test\r\ntest"
+ val s2 = "test\ntest"
+ val s3 = "test\rtest"
+ val s4 = "testtest"
+
+ assert(EOLUtils.convertToSystemEOL(s1) === EOLUtils.convertToSystemEOL(s2))
+ assert(EOLUtils.convertToSystemEOL(s1) === EOLUtils.convertToSystemEOL(s3))
+ assert(EOLUtils.convertToSystemEOL(s2) === EOLUtils.convertToSystemEOL(s3))
+ assert(EOLUtils.convertToSystemEOL(s4) === s4)
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/8027ca70/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala
----------------------------------------------------------------------
diff --git a/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala b/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala
index ff29d95..853290a 100644
--- a/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala
+++ b/integration-test/src/test/scala/org/apache/livy/test/InteractiveIT.scala
@@ -97,6 +97,7 @@ class InteractiveIT extends BaseIntegrationTestSuite {
s.run("%table x").verifyResult(".*headers.*type.*name.*data.*")
s.run("abcde").verifyError(ename = "NameError", evalue = "name 'abcde' is not defined")
s.run("raise KeyError, 'foo'").verifyError(ename = "KeyError", evalue = "'foo'")
+ s.run("print(1)\r\nprint(1)").verifyResult("1\n1")
}
}
@@ -115,6 +116,7 @@ class InteractiveIT extends BaseIntegrationTestSuite {
"""|root
| |-- name: string (nullable = true)
| |-- age: double (nullable = true)""".stripMargin))
+ s.run("print(1)\r\nprint(1)").verifyResult(".*1\n.*1")
}
}
http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/8027ca70/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala
----------------------------------------------------------------------
diff --git a/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala b/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala
index af51e43..b805a4d 100644
--- a/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala
+++ b/repl/src/main/scala/org/apache/livy/repl/ReplDriver.scala
@@ -23,7 +23,7 @@ import scala.concurrent.duration.Duration
import io.netty.channel.ChannelHandlerContext
import org.apache.spark.SparkConf
-import org.apache.livy.Logging
+import org.apache.livy.{EOLUtils, Logging}
import org.apache.livy.client.common.ClientConf
import org.apache.livy.rsc.{BaseProtocol, ReplJobResults, RSCConf}
import org.apache.livy.rsc.BaseProtocol.ReplState
@@ -55,7 +55,7 @@ class ReplDriver(conf: SparkConf, livyConf: RSCConf)
}
def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.ReplJobRequest): Int = {
- session.execute(msg.code, msg.codeType)
+ session.execute(EOLUtils.convertToSystemEOL(msg.code), msg.codeType)
}
def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.CancelReplJobRequest): Unit = {
@@ -63,7 +63,7 @@ class ReplDriver(conf: SparkConf, livyConf: RSCConf)
}
def handle(ctx: ChannelHandlerContext, msg: BaseProtocol.ReplCompleteRequest): Array[String] = {
- session.complete(msg.code, msg.codeType, msg.cursor)
+ session.complete(EOLUtils.convertToSystemEOL(msg.code), msg.codeType, msg.cursor)
}
/**