Skip to content

[LIVY-414] Support environment variables for interactive and batch sessions#80

Closed
risdenk wants to merge 1 commit into
apache:masterfrom
risdenk:LIVY-414
Closed

[LIVY-414] Support environment variables for interactive and batch sessions#80
risdenk wants to merge 1 commit into
apache:masterfrom
risdenk:LIVY-414

Conversation

@risdenk

@risdenk risdenk commented Mar 3, 2018

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/LIVY-414

What changes were proposed in this pull request?

How was this patch tested?

  • New unittest to ensure that SparkProcessBuilder gets environment variables before launching spark-submit
  • New unittest to ensure that SparkLauncher gets environment variables before creating session
  • Batch sessions tested against Spark cluster

@risdenk

risdenk commented Mar 3, 2018

Copy link
Copy Markdown
Contributor Author

Test failures:

[INFO] --- scalastyle-maven-plugin:0.8.0:check (default) @ livy-server ---
error file=/home/travis/build/apache/incubator-livy/server/src/test/scala/org/apache/livy/server/batch/BatchSessionSpec.scala message=import.ordering.missingEmptyLine.message line=25 column=0
error file=/home/travis/build/apache/incubator-livy/server/src/test/scala/org/apache/livy/server/batch/BatchSessionSpec.scala message=import.ordering.missingEmptyLine.message line=30 column=0

Will fix in a few minutes once all tests finish running locally.

@codecov-io

codecov-io commented Mar 3, 2018

Copy link
Copy Markdown

Codecov Report

Merging #80 into master will increase coverage by 0.12%.
The diff coverage is 90.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #80      +/-   ##
===========================================
+ Coverage     71.47%   71.6%   +0.12%     
- Complexity      793     800       +7     
===========================================
  Files            97      97              
  Lines          5396    5413      +17     
  Branches        798     804       +6     
===========================================
+ Hits           3857    3876      +19     
+ Misses         1015    1011       -4     
- Partials        524     526       +2
Impacted Files Coverage Δ Complexity Δ
...main/java/org/apache/livy/rsc/ContextLauncher.java 84.76% <100%> (+0.44%) 20 <0> (+2) ⬆️
.../server/interactive/CreateInteractiveRequest.scala 81.25% <100%> (+1.25%) 23 <1> (+2) ⬆️
.../apache/livy/server/batch/CreateBatchRequest.scala 67.64% <100%> (+2.02%) 20 <1> (+2) ⬆️
rsc/src/main/java/org/apache/livy/rsc/RSCConf.java 87.85% <100%> (+0.11%) 8 <0> (ø) ⬇️
...la/org/apache/livy/server/batch/BatchSession.scala 85.55% <50%> (-0.97%) 13 <0> (ø)
...e/livy/server/interactive/InteractiveSession.scala 67.58% <80%> (+0.19%) 42 <0> (ø) ⬇️
...in/java/org/apache/livy/rsc/rpc/RpcDispatcher.java 67% <0%> (-1%) 20% <0%> (ø)
...ain/java/org/apache/livy/rsc/driver/RSCDriver.java 80% <0%> (+0.85%) 42% <0%> (ø) ⬇️
...la/org/apache/livy/utils/SparkProcessBuilder.scala 60% <0%> (+3.33%) 13% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10373b6...3af2252. Read the comment docs.

(if (queue.isDefined) s"queue: ${queue.get}, " else "") +
(if (name.isDefined) s"name: ${name.get}, " else "") +
(if (conf.nonEmpty) s"conf: ${conf.mkString(",")}]" else "]")
(if (conf.nonEmpty) s"conf: ${conf.mkString(",")}]" else "]") +

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll review more later, but quick nit: the ]s here needs to be removed since the line below now adds them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks good catch. I'll fix in a minute.

@risdenk

risdenk commented Mar 3, 2018

Copy link
Copy Markdown
Contributor Author

Some improvements that might be necessary:

  • whitelist for supported environment variables
    • currently it would be possible to set PATH before spark-submit
    • not sure this is completely secure
    • could whitelist just SPARK_HOME and SPARK_CLASSPATH to start
    • an empty whitelist would disable this feature
  • ensure that SPARK_CLASSPATH is only set when Spark is set to run in cluster mode
    • if running in client mode, spark.driver.extraClasspath controls the spark-submit classpath
    • Users will get an error if SPARK_CLASSPATH is set when running in client mode

@jerryshao

Copy link
Copy Markdown
Contributor

@risdenk Why do we only support batch session, I think it is also needed for interactive session, right?

@risdenk

risdenk commented Mar 12, 2018

Copy link
Copy Markdown
Contributor Author

Manually tested the current PR with changing SPARK_HOME during the request.

Setup

  • Set livy.file.local-dir-whitelist = /home/USER/test_livy in livy.conf
  • export SPARK_HOME=/home/USER/sparks/spark-1.6.3-bin-hadoop2.6

Simple Python test file

from pyspark import SparkContext
sc = SparkContext()
print('Spark Version: ' + sc.version)

Testing different SPARK_HOME
SPARK_HOME not set

curl -i -XPOST http://$(hostname -f):8998/batches -H 'Content-Type: application/json' -d '{"file": "/home/USER/test_livy/livy.py"}'
# Spark Version: 1.6.3

SPARK_HOME set to Spark 1.6.3

curl -i -XPOST http://$(hostname -f):8998/batches -H 'Content-Type: application/json' -d '{"file": "/home/USER/test_livy/livy.py", "env" : {"SPARK_HOME": "/home/USER/sparks/spark-1.6.3-bin-hadoop2.6"}}'
# Spark Version: 1.6.3

SPARK_HOME set to Spark 2.2.1

curl -i -XPOST http://$(hostname -f):8998/batches -H 'Content-Type: application/json' -d '{"file": "/home/USERNAME/test_livy/livy.py", "env" : {"SPARK_HOME": "/home/USER/sparks/spark-2.2.1-bin-hadoop2.7"}}'
# Spark Version: 2.2.1

SPARK_HOME set to Spark 2.3.0

curl -i -XPOST http://$(hostname -f):8998/batches -H 'Content-Type: application/json' -d '{"file": "/home/USER/test_livy/livy.py", "env" : {"SPARK_HOME": "/home/USER/sparks/spark-2.3.0-bin-hadoop2.7"}}'
# Spark Version: 2.3.0

@risdenk

risdenk commented Mar 12, 2018

Copy link
Copy Markdown
Contributor Author

@jerryshao - I only needed it for batch sessions currently so started with that. If you or someone has pointers on where to start adding to interactive session I can look at that too.

@jerryshao

Copy link
Copy Markdown
Contributor

For the completeness of PR, I think adding support of interactive session is also required.

@risdenk risdenk changed the title [LIVY-414] Support environment variables for batch sessions [LIVY-414] Support environment variables for interactive and batch sessions Mar 14, 2018
@risdenk

risdenk commented Mar 14, 2018

Copy link
Copy Markdown
Contributor Author

@jerryshao - I took a stab at adding environment variable support for interactive sessions as well.

…ssions

Signed-off-by: Kevin Risden <krisden@apache.org>
@risdenk

risdenk commented Nov 18, 2018

Copy link
Copy Markdown
Contributor Author

Rebased on current master.

@risdenk

risdenk commented Nov 18, 2018

Copy link
Copy Markdown
Contributor Author

Test failures are unrelated to this change: on project livy-thriftserver: There are test failures

@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has had no activity for at least 3 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Livy!

@github-actions github-actions Bot added the stale label Apr 28, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Closing this pull request due to at least 4 months of inactivity. If you would like to continue the work, please feel free to reopen this pull request or open a new one.

@github-actions github-actions Bot closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants