[LIVY-498][REPL] Fix Windows CRLF line ending issue in SparkR interpreter #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #105 +/- ##
============================================
- Coverage 71.4% 71.28% -0.13%
+ Complexity 796 795 -1
============================================
Files 97 98 +1
Lines 5476 5519 +43
Branches 816 832 +16
============================================
+ Hits 3910 3934 +24
- Misses 1036 1051 +15
- Partials 530 534 +4
Continue to review full report at Codecov.
|
|
@ajbozarth @zjffdu would you please help to review, thanks! |
zjffdu
reviewed
Aug 20, 2018
|
|
||
| def convertToOldMacEOL(s: String): String = convertLineEndings(s, EOL_OLD_MAC) | ||
|
|
||
| def convertToSystemEOL(s: String): String = convertLineEndings(s, EOL_SYSTEM_DEFAULT) |
There was a problem hiding this comment.
Do we need other covert method ? I see only this method is used by livy
There was a problem hiding this comment.
Yes, this is the one only required, I can remove others if you think they're not necessary.
There was a problem hiding this comment.
Was your intent by having all these options to reuse this file in other places? because if it's just for this one Livy use case then this file is a bit excessive.
|
The code LGTM, but I don't have easy access to a windows machine to test it out though (my team is all MacBooks and linux VMs) |
|
I'm going to merge this if there's no other comment, thanks! |
|
Merging to master and 0.5. Thanks for the review. |
asfgit
pushed a commit
that referenced
this pull request
Aug 22, 2018
…eter ## 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 <[email protected]> Closes #105 from jerryshao/LIVY-498. (cherry picked from commit 8027ca7) Signed-off-by: jerryshao <[email protected]>
tkakantousis
pushed a commit
to tkakantousis/incubator-livy
that referenced
this pull request
Oct 23, 2018
…eter ## 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 <[email protected]> Closes apache#105 from jerryshao/LIVY-498. (cherry picked from commit 8027ca7) Signed-off-by: jerryshao <[email protected]>
tkakantousis
added a commit
to logicalclocks/incubator-livy
that referenced
this pull request
Oct 24, 2018
* [MINOR] Addressed Docs dependency security vulnerability followup Followup to 26428c5 Author: Alex Bozarth <[email protected]> Closes apache#82 from ajbozarth/gem. (cherry picked from commit 6d2ffdd) Signed-off-by: Alex Bozarth <[email protected]> * [MINOR] Fix travis builds Each individual commit has a more detailed description of what's being changed and why. At the moment, Travis builds don't work: * https://travis-ci.org/mineo/incubator-livy/builds/359324523 - the `sudo pip3 install --upgrade pip "setuptools < 36"` command fails with `sudo: pip3: command not found`. * fixing that, the `failing the org.apache.rat:apache-rat-plugin:0.12:check` maven goal fails because it sees `.pytest_cache` folders that it doesn't know about (pytest-dev/pytest#3286, failure in https://travis-ci.org/mineo/incubator-livy/jobs/359326261, check the raw log). This pull request works around the pip3 failures by just using pip as a callable module (possible since Python 3.4) and adding `.pytest_cache` to raw-excludes, as well as showing the contents of `rat.txt` files in the `after_failure` step. I concede that the pip change is more of a workaround, but I don't know how else to fix it and unblock testing. Running the tests on travis. Please review https://livy.incubator.apache.org/community/ before opening a pull request. Author: Wieland Hoffmann <[email protected]> Closes apache#84 from mineo/rat. (cherry picked from commit 06cfa7b) Signed-off-by: jerryshao <[email protected]> * [LIVY-455][REPL] Fix json4s doesn't support java.math.BigDecimal issue ## What changes were proposed in this pull request? Livy's SQLInterpreter will throw exception when rows contain java.math.BigDecimal data. This is because current version of json4s doesn't treat java.math.BigDecimal type as primitive type. On the contrary, json4s supports Scala BigDecimal as primitive type. So the fix is to convert java BigDecimal to Scala BigDecimal. ## How was this patch tested? Unit test is added. Author: jerryshao <[email protected]> Closes apache#85 from jerryshao/LIVY-455. (cherry picked from commit 7e4bb3b) Signed-off-by: jerryshao <[email protected]> * [LIVY-457][REPL] Fix SQLContext is not initialized correctly issue ## What changes were proposed in this pull request? The signature of SQLContext's constructor is changed in Spark2, but we're still using the Spark1's signature, which will throw an exception when using this object. ## How was this patch tested? UT and local verification. Author: jerryshao <[email protected]> Closes apache#86 from jerryshao/LIVY-457. (cherry picked from commit cd8b112) Signed-off-by: jerryshao <[email protected]> * [LIVY-466][RSC] Fix RSCDriver exception during RPC shutdown ## What changes were proposed in this pull request? During RSCDriver's shutdown, it will first shutdown RPC server, and then all the RPC clients. When RPC client is closed, it will register a timeout to avoid orphaned RSCDriver, but this is not necessary during RSCDriver's shutdown, so here fixing this issue. The details can be seen in [JIRA](https://issues.apache.org/jira/browse/LIVY-466). ## How was this patch tested? Local verification. Author: jerryshao <[email protected]> Closes apache#90 from jerryshao/LIVY-466. (cherry picked from commit e3f45a0) Signed-off-by: jerryshao <[email protected]> * [LIVY-472][SERVER] Improve the logs for fail-to-create session ## What changes were proposed in this pull request? Livy currently doesn't give a very clear log about the fail-to-create session, it only says that session related app tag cannot be found in RM, but doesn't tell user how to search and get the true root cause. So here change the logs to make it more clear. ## How was this patch tested? Local verification. Author: jerryshao <[email protected]> Closes apache#96 from jerryshao/LIVY-472. (cherry picked from commit ca4cad2) Signed-off-by: jerryshao <[email protected]> * [Security] Update to support pyspark and sparkr changes in Spark 2.3.1 * [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 <[email protected]> Closes apache#105 from jerryshao/LIVY-498. (cherry picked from commit 8027ca7) Signed-off-by: jerryshao <[email protected]> * upgrade to work with spark 2.3.2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.