You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@livy.apache.org by vanzin <gi...@git.apache.org> on 2018/09/21 18:39:49 UTC
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
GitHub user vanzin opened a pull request:
https://github.com/apache/incubator-livy/pull/112
[LIVY-511][LIVY-512] Remove support for old Spark, Scala versions.
This change restricts Livy support to Spark 2.2+ and Scala 2.11. Both changes
are made together because by supporting Spark 2.2+ only, it becomes impossible
to test Scala 2.10.
As part of the change, a lot of code that used reflection to support different
versions of Spark could be cleaned up and directly call Spark APIs.
The Scala 2.10 parts of the builds also have been removed, but the actual
support for building and running with different Scala versions (and related
tests) have been left as is. This will allow us to support 2.12 in the future.
This change intentionally does not touch the public API (the "api/" module).
There are things that could be cleaned up now that Spark 1.x is not supported,
but that would mean an API breakage so I chose to leave those alone for now.
The test matrix and build profiles have also been simplified a lot. There are
now two profiles to choose from (for Spark 2.2 and 2.3); integration tests can
be run against a different version of Spark by running just the integration
test module with the desired profile.
Tested with Spark 2.2 and 2.3, and also by building against 2.2 and running
integration tests against 2.3.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vanzin/incubator-livy LIVY-511
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-livy/pull/112.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #112
----
commit 6459626488a050935e25cceafe4e8b22e91e3012
Author: Marcelo Vanzin <va...@...>
Date: 2018-09-20T18:39:23Z
[LIVY-511][LIVY-512] Remove support for old Spark, Scala versions.
This change restricts Livy support to Spark 2.2+ and Scala 2.11. Both changes
are made together because by supporting Spark 2.2+ only, it becomes impossible
to test Scala 2.10.
As part of the change, a lot of code that used reflection to support different
versions of Spark could be cleaned up and directly call Spark APIs.
The Scala 2.10 parts of the builds also have been removed, but the actual
support for building and running with different Scala versions (and related
tests) have been left as is. This will allow us to support 2.12 in the future.
This change intentionally does not touch the public API (the "api/" module).
There are things that could be cleaned up now that Spark 1.x is not supported,
but that would mean an API breakage so I chose to leave those alone for now.
The test matrix and build profiles have also been simplified a lot. There are
now two profiles to choose from (for Spark 2.2 and 2.3); integration tests can
be run against a different version of Spark by running just the integration
test module with the desired profile.
Tested with Spark 2.2 and 2.3, and also by building against 2.2 and running
integration tests against 2.3.
----
---
[GitHub] incubator-livy issue #112: [LIVY-511][LIVY-512] Remove support for old Spark...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/incubator-livy/pull/112
Any more comments? I'd like this to get in to unblock my changes for LIVY-503 (which right now depend on Spark 2.2).
---
[GitHub] incubator-livy issue #112: [LIVY-511][LIVY-512] Remove support for old Spark...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/incubator-livy/pull/112
@vanzin thanks, I have no more comments apart from https://github.com/apache/incubator-livy/pull/112#discussion_r220647323. I have not reviewed it carefully, but I am still quite new to Livy codebase, so it would take a lot for me to do a throughout review of this PR, which is pretty huge.
So once you address that comment I am fine with this PR, thanks.
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin closed the pull request at:
https://github.com/apache/incubator-livy/pull/112
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by vanzin <gi...@git.apache.org>.
GitHub user vanzin reopened a pull request:
https://github.com/apache/incubator-livy/pull/112
[LIVY-511][LIVY-512] Remove support for old Spark, Scala versions.
This change restricts Livy support to Spark 2.2+ and Scala 2.11. Both changes
are made together because by supporting Spark 2.2+ only, it becomes impossible
to test Scala 2.10.
As part of the change, a lot of code that used reflection to support different
versions of Spark could be cleaned up and directly call Spark APIs.
The Scala 2.10 parts of the builds also have been removed, but the actual
support for building and running with different Scala versions (and related
tests) have been left as is. This will allow us to support 2.12 in the future.
This change intentionally does not touch the public API (the "api/" module).
There are things that could be cleaned up now that Spark 1.x is not supported,
but that would mean an API breakage so I chose to leave those alone for now.
The test matrix and build profiles have also been simplified a lot. There are
now two profiles to choose from (for Spark 2.2 and 2.3); integration tests can
be run against a different version of Spark by running just the integration
test module with the desired profile.
Tested with Spark 2.2 and 2.3, and also by building against 2.2 and running
integration tests against 2.3.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vanzin/incubator-livy LIVY-511
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-livy/pull/112.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #112
----
commit 6459626488a050935e25cceafe4e8b22e91e3012
Author: Marcelo Vanzin <va...@...>
Date: 2018-09-20T18:39:23Z
[LIVY-511][LIVY-512] Remove support for old Spark, Scala versions.
This change restricts Livy support to Spark 2.2+ and Scala 2.11. Both changes
are made together because by supporting Spark 2.2+ only, it becomes impossible
to test Scala 2.10.
As part of the change, a lot of code that used reflection to support different
versions of Spark could be cleaned up and directly call Spark APIs.
The Scala 2.10 parts of the builds also have been removed, but the actual
support for building and running with different Scala versions (and related
tests) have been left as is. This will allow us to support 2.12 in the future.
This change intentionally does not touch the public API (the "api/" module).
There are things that could be cleaned up now that Spark 1.x is not supported,
but that would mean an API breakage so I chose to leave those alone for now.
The test matrix and build profiles have also been simplified a lot. There are
now two profiles to choose from (for Spark 2.2 and 2.3); integration tests can
be run against a different version of Spark by running just the integration
test module with the desired profile.
Tested with Spark 2.2 and 2.3, and also by building against 2.2 and running
integration tests against 2.3.
commit 7e8b6864a375d17b8a374ea0e81cef419f32a510
Author: Marcelo Vanzin <va...@...>
Date: 2018-09-26T17:13:14Z
Break unit tests and ITs into separate jobs.
----
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220647323
--- Diff: .travis.yml ---
@@ -19,24 +19,12 @@ sudo: required
dist: trusty
language: scala
-env:
- matrix:
- - MVN_FLAG='-DskipTests'
- - MVN_FLAG='-Pspark-2.0-it -DskipTests'
- - MVN_FLAG='-Pspark-2.1-it -DskipTests'
- - MVN_FLAG='-Pspark-1.6 -DskipITs'
- - MVN_FLAG='-Pspark-2.0 -DskipITs'
- - MVN_FLAG='-Pspark-2.1 -DskipITs'
-
matrix:
include:
- # Spark 2.2+ will only be verified using JDK8
- # Thriftserver requires JDK8
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2 -DskipITs'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3 -DskipITs'
-
+ - name: "Spark 2.2 + ITs"
+ env: MVN_FLAG='-Pthriftserver'
--- End diff --
Seems like the limit is 50m, so we are pretty close to it: https://docs.travis-ci.com/user/customizing-the-build/#build-timeouts.
Can we have separated builds? Thanks.
---
[GitHub] incubator-livy issue #112: [LIVY-511][LIVY-512] Remove support for old Spark...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/incubator-livy/pull/112
thanks @vanzin
---
[GitHub] incubator-livy issue #112: [LIVY-511][LIVY-512] Remove support for old Spark...
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/incubator-livy/pull/112
# [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=h1) Report
> Merging [#112](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/d39ab356b4e36d8780bd44b9d5020bfeec90cb2f?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `57.14%`.
[](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #112 +/- ##
============================================
+ Coverage 71.12% 71.12% +<.01%
- Complexity 796 928 +132
============================================
Files 98 99 +1
Lines 5500 5483 -17
Branches 829 828 -1
============================================
- Hits 3912 3900 -12
- Misses 1056 1059 +3
+ Partials 532 524 -8
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../scala/org/apache/livy/repl/SparkInterpreter.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cmVwbC9zY2FsYS0yLjExL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9yZXBsL1NwYXJrSW50ZXJwcmV0ZXIuc2NhbGE=) | `82.97% <ø> (ø)` | `10 <0> (?)` | |
| [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `93.79% <ø> (ø)` | `21 <0> (ø)` | :arrow_down: |
| [...la/org/apache/livy/server/batch/BatchSession.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvYmF0Y2gvQmF0Y2hTZXNzaW9uLnNjYWxh) | `85.22% <ø> (-0.17%)` | `13 <0> (ø)` | |
| [...main/java/org/apache/livy/rsc/ContextLauncher.java](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9Db250ZXh0TGF1bmNoZXIuamF2YQ==) | `82.65% <100%> (-1.27%)` | `22 <0> (+4)` | |
| [...in/scala/org/apache/livy/repl/SQLInterpreter.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cmVwbC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvcmVwbC9TUUxJbnRlcnByZXRlci5zY2FsYQ==) | `70.37% <100%> (-2.61%)` | `7 <1> (+6)` | |
| [...n/scala/org/apache/livy/utils/LivySparkUtils.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS91dGlscy9MaXZ5U3BhcmtVdGlscy5zY2FsYQ==) | `67.6% <100%> (-4.02%)` | `0 <0> (ø)` | |
| [...e/livy/server/interactive/InteractiveSession.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvaW50ZXJhY3RpdmUvSW50ZXJhY3RpdmVTZXNzaW9uLnNjYWxh) | `69.2% <25%> (+2.02%)` | `43 <0> (ø)` | :arrow_down: |
| [.../java/org/apache/livy/rsc/driver/SparkEntries.java](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvU3BhcmtFbnRyaWVzLmphdmE=) | `64.06% <33.33%> (-0.33%)` | `10 <0> (-1)` | |
| [...ava/org/apache/livy/rsc/driver/JobContextImpl.java](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvSm9iQ29udGV4dEltcGwuamF2YQ==) | `91.11% <66.66%> (ø)` | `17 <0> (ø)` | :arrow_down: |
| [...rg/apache/livy/repl/AbstractSparkInterpreter.scala](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree#diff-cmVwbC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvcmVwbC9BYnN0cmFjdFNwYXJrSW50ZXJwcmV0ZXIuc2NhbGE=) | `52.5% <0%> (-4.3%)` | `30% <0%> (+29%)` | |
| ... and [29 more](https://codecov.io/gh/apache/incubator-livy/pull/112/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=footer). Last update [d39ab35...6459626](https://codecov.io/gh/apache/incubator-livy/pull/112?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-livy/pull/112
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220122661
--- Diff: .travis.yml ---
@@ -19,24 +19,12 @@ sudo: required
dist: trusty
language: scala
-env:
- matrix:
- - MVN_FLAG='-DskipTests'
- - MVN_FLAG='-Pspark-2.0-it -DskipTests'
- - MVN_FLAG='-Pspark-2.1-it -DskipTests'
- - MVN_FLAG='-Pspark-1.6 -DskipITs'
- - MVN_FLAG='-Pspark-2.0 -DskipITs'
- - MVN_FLAG='-Pspark-2.1 -DskipITs'
-
matrix:
include:
- # Spark 2.2+ will only be verified using JDK8
- # Thriftserver requires JDK8
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2 -DskipITs'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3 -DskipITs'
-
+ - name: "Spark 2.2 + ITs"
+ env: MVN_FLAG='-Pthriftserver'
--- End diff --
I remember a comment by @jerryshao when he mentioned that there is a timeout for a single build in travis but we have not limit on the number of runs. If this is the case, what about restoring having one build with `-DskipTests` and one with `-DskipITs` in order to reduce the time of each of them?
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220259542
--- Diff: .travis.yml ---
@@ -19,24 +19,12 @@ sudo: required
dist: trusty
language: scala
-env:
- matrix:
- - MVN_FLAG='-DskipTests'
- - MVN_FLAG='-Pspark-2.0-it -DskipTests'
- - MVN_FLAG='-Pspark-2.1-it -DskipTests'
- - MVN_FLAG='-Pspark-1.6 -DskipITs'
- - MVN_FLAG='-Pspark-2.0 -DskipITs'
- - MVN_FLAG='-Pspark-2.1 -DskipITs'
-
matrix:
include:
- # Spark 2.2+ will only be verified using JDK8
- # Thriftserver requires JDK8
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.2 -DskipITs'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3-it -DskipTests'
- - env: MVN_FLAG='-Pthriftserver -Pspark-2.3 -DskipITs'
-
+ - name: "Spark 2.2 + ITs"
+ env: MVN_FLAG='-Pthriftserver'
--- End diff --
The builds seem to be taking a little bit over 40m. If that's close to the timeout we can break them down.
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220259713
--- Diff: README.md ---
@@ -57,12 +57,8 @@ Required python packages for building Livy:
To run Livy, you will also need a Spark installation. You can get Spark releases at
https://spark.apache.org/downloads.html.
-Livy requires at least Spark 1.6 and supports both Scala 2.10 and 2.11 builds of Spark, Livy
-will automatically pick repl dependencies through detecting the Scala version of Spark.
-
-Livy also supports Spark 2.0+ for both interactive and batch submission, you could seamlessly
-switch to different versions of Spark through ``SPARK_HOME`` configuration, without needing to
-rebuild Livy.
+Livy requires Spark 2.2 or 2.3. You can switch to a different version of Spark by setting the
--- End diff --
The code explicitly leaves 2.4 out. In fact, even if it didn't, 2.4 wouldn't work because the REPL API has changed.
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220264313
--- Diff: README.md ---
@@ -57,12 +57,8 @@ Required python packages for building Livy:
To run Livy, you will also need a Spark installation. You can get Spark releases at
https://spark.apache.org/downloads.html.
-Livy requires at least Spark 1.6 and supports both Scala 2.10 and 2.11 builds of Spark, Livy
-will automatically pick repl dependencies through detecting the Scala version of Spark.
-
-Livy also supports Spark 2.0+ for both interactive and batch submission, you could seamlessly
-switch to different versions of Spark through ``SPARK_HOME`` configuration, without needing to
-rebuild Livy.
+Livy requires Spark 2.2 or 2.3. You can switch to a different version of Spark by setting the
--- End diff --
I see, thanks for the explaination.
---
[GitHub] incubator-livy issue #112: [LIVY-511][LIVY-512] Remove support for old Spark...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/incubator-livy/pull/112
ok, merging to master.
---
[GitHub] incubator-livy pull request #112: [LIVY-511][LIVY-512] Remove support for ol...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/incubator-livy/pull/112#discussion_r220122923
--- Diff: README.md ---
@@ -57,12 +57,8 @@ Required python packages for building Livy:
To run Livy, you will also need a Spark installation. You can get Spark releases at
https://spark.apache.org/downloads.html.
-Livy requires at least Spark 1.6 and supports both Scala 2.10 and 2.11 builds of Spark, Livy
-will automatically pick repl dependencies through detecting the Scala version of Spark.
-
-Livy also supports Spark 2.0+ for both interactive and batch submission, you could seamlessly
-switch to different versions of Spark through ``SPARK_HOME`` configuration, without needing to
-rebuild Livy.
+Livy requires Spark 2.2 or 2.3. You can switch to a different version of Spark by setting the
--- End diff --
Is there a specific reason not to state just Spark 2.2+? Do we have problems with 2.4?
---