You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2020/09/28 01:59:24 UTC

[GitHub] [incubator-yunikorn-k8shim] yanghua opened a new pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

yanghua opened a new pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193


   jira detail: https://issues.apache.org/jira/browse/YUNIKORN-426


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...18dcf14](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495692884



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`

Review comment:
       happy with that too




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on pull request #193: [YUNIKORN-426] Refactor Spark deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-701111068


   @wilfred-s Any opinion?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495666540



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then
+  echo "The checksum is matched!"
+  rm -rf $WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}

Review comment:
       I thought about this problem, but we cannot guarantee whether the existing folder has been changed. If it is unpredictable, why not just delete it based on the same standard file first. The decompression process does not consume more time.
   
   Here, I think our goal is to let new users experience it. If it is not sufficiently automated and requires too high learning costs, then it will bring some rebellious emotions.
   
   WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...18dcf14](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495665522



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`

Review comment:
       We can give a parameter to let the user specify the location.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495657079



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then

Review comment:
       If you want to check the checksum you must also pull down the checksum from the source.
   Don't store the checksum in our code. If someone decides to change the version to 3.0.1 in the script this will break.

##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`

Review comment:
       This forces a run from within the checked out tree. Which means you remove the flexibility from the user to have or use multiple inputs and setups.
   The user should have the flexibility to run to run this from wherever they want on the system.

##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7

Review comment:
       See below, since you do not download the checksum file this is really still fixed to one specific version.
   We should also be able to pick up these values from outside the script, for both spark and hadoop. Something like this:
   ```
   if [ "$SPARK_VERSION" = "" ]; then
       echo using default Spark version;
       SPARK_VERSION=3.0.0
   fi
   ```

##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then
+  echo "The checksum is matched!"
+  rm -rf $WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}

Review comment:
       If we have an unpacked version already why do we not detect that and use it instead of checking for the compressed archive?

##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then
+  echo "The checksum is matched!"
+  rm -rf $WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+  tar -xvzf $SPARK_BINARY_FILE_PATH -C $WORK_SPACE_ROOT
+else
+  echo "The checksum is not matched, Removing the incompleted file, please download it again."
+  rm -f ./binarycache/${SPARK_BINARY_FILE_NAME}

Review comment:
       This is the first mention of a `binarycache` directory, this will never work.

##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}

Review comment:
       checksum and binary if you want to auto download




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495694075



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then
+  echo "The checksum is matched!"
+  rm -rf $WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}

Review comment:
       Make the behaviour clear, always using a clean unpacked version is OK but the user needs to be made aware that you are doing it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...bbec71d](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...bfdf4bf](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495668312



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7

Review comment:
       But we have `spark24` demo dir. Maybe we can merge these two isolated demos through variables?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] commented on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #193   +/-   ##
   =======================================
     Coverage   58.33%   58.33%           
   =======================================
     Files          33       33           
     Lines        2933     2933           
   =======================================
     Hits         1711     1711           
     Misses       1150     1150           
     Partials       72       72           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...90475b4](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...bbec71d](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s closed pull request #193: [YUNIKORN-426] Refactor Spark deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699854896


    @wilfred-s I have addressed most of the suggestions and run the script to submit on the K8S successfully. Please review.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495692795



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then

Review comment:
       The offical spark checksum is generated using this command:
   ```
   gpg --print-md sha512 spark-3.0.1-bin-hadoop2.7.tgz
   spark-3.0.1-bin-hadoop2.7.tgz: F4A10BAE C5B8FF18 41F10651 CAC2C4AA 39C162D3
                                  029CA180 A9749149 E6060805 B5B5DDF9 287B4AA3
                                  21434810 172F8CC0 534943AC 005531BB 48B6622F
                                  BE228DDC
   ```
   There are really only two versions: `gpg` and `shasum`
   
   It is also really simple to generate the format you want using:
   ```
   cat spark-3.0.1-bin-hadoop2.7.tgz.sha512 | tr -d " \t\n\r" | awk -F: '{print $2 "  " $1}' > converted.sha512
   ```
   That output is the same as the shasum output. Redirect into a file and use `shasum -c -a 512 converted.sha512` and you are good to go




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495687081



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7

Review comment:
       Sure that would be nice, just one version to maintain.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699854896


    @wilfred-s I have addressed most of the suggestions. Please review.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #193   +/-   ##
   =======================================
     Coverage   58.33%   58.33%           
   =======================================
     Files          33       33           
     Lines        2933     2933           
   =======================================
     Hits         1711     1711           
     Misses       1150     1150           
     Partials       72       72           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...90475b4](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #193      +/-   ##
   ==========================================
   - Coverage   58.33%   58.26%   -0.07%     
   ==========================================
     Files          33       33              
     Lines        2933     2933              
   ==========================================
   - Hits         1711     1709       -2     
   - Misses       1150     1152       +2     
     Partials       72       72              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `81.81% <0.00%> (-2.03%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...bbec71d](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699741695


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=h1) Report
   > Merging [#193](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/33956f00eed9ee5afebe1239ef33d8515956ec3d?el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #193   +/-   ##
   =======================================
     Coverage   58.33%   58.33%           
   =======================================
     Files          33       33           
     Lines        2933     2933           
   =======================================
     Hits         1711     1711           
     Misses       1150     1150           
     Partials       72       72           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?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-yunikorn-k8shim/pull/193?src=pr&el=footer). Last update [33956f0...18dcf14](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#issuecomment-699728876


   cc @yangwwei 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495664815



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then

Review comment:
       @wilfred-s I have tried to download the checksum from the official website. But there are two issues:
   
   - We can not make sure the checksum file is completeness.
   - The official sha512 file is more complex, e.g. blank and capital and small letter, we need to spend more time to verify them. Considering that we will have more framework demos in the future, each checksum file is different.
   
   It seems we can just copy the checksum code and store it into a variable. I saw Flink did this. WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-yunikorn-k8shim] yanghua commented on a change in pull request #193: [YUNIKORN-426] Refactor Spark v3.0 deployment demo to remove some hardcode

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #193:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/193#discussion_r495701421



##########
File path: deployments/examples/spark30/cmd/run.sh
##########
@@ -16,12 +16,40 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# replace these env var values accordingly
-SPARK_HOME=/Users/example/repository/spark
+SCRIPT_PATH=$(cd `dirname $0`; pwd)
+# set up root directory
+WORK_SPACE_ROOT=`dirname $SCRIPT_PATH`
+
+SPARK_VERSION=3.0.0
+SPARK_HADOOP_VERSION=2.7
+
+SPARK_BINARY_FILE_NAME=spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}.tgz
+SPARK_BINARY_CHECKSUM_FILE_NAME=${SPARK_BINARY_FILE_NAME}.sha512
+SPARK_BINARY_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_FILE_NAME
+SPARK_BINARY_CHECKSUM_FILE_PATH=$WORK_SPACE_ROOT/$SPARK_BINARY_CHECKSUM_FILE_NAME
+SPARK_HOME=$WORK_SPACE_ROOT/spark-${SPARK_VERSION}-bin-hadoop${SPARK_HADOOP_VERSION}
+SPARK_EXAMPLE_JAR=local://$SPARK_HOME/examples/jars/spark-examples_2.12-${SPARK_VERSION}.jar
+
 K8S_ENDPOINT=http://localhost:8001
 SPARK_EXECUTOR_NUM=1
 SPARK_DOCKER_IMAGE=yunikorn/spark:latest
-SPARK_EXAMPLE_JAR=local:///opt/spark/examples/jars/spark-examples_2.12-3.0.0-SNAPSHOT.jar
+
+if [ -f $SPARK_BINARY_FILE_PATH ]; then
+  echo "The binary file $SPARK_BINARY_FILE_NAME has been cached!"
+else
+  echo "The binary file $SPARK_BINARY_FILE_NAME did not exist, try to download."
+  wget http://archive.apache.org/dist/spark/spark-${SPARK_VERSION}/${SPARK_BINARY_FILE_NAME} -O ${SPARK_BINARY_FILE_PATH}
+fi
+
+# check signature to verify the completeness
+if [[ $(cat $SPARK_BINARY_CHECKSUM_FILE_PATH) == $(shasum -a 512 $SPARK_BINARY_FILE_PATH | awk '{print $1}') ]]; then

Review comment:
       OK, let me try.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org