Github user liyinan926 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22608#discussion_r225718039
  
    --- Diff: bin/docker-image-tool.sh ---
    @@ -71,18 +71,29 @@ function build {
         --build-arg
         base_img=$(image_ref spark)
       )
    -  local BASEDOCKERFILE=${BASEDOCKERFILE:-"$IMG_PATH/spark/Dockerfile"}
    -  local 
PYDOCKERFILE=${PYDOCKERFILE:-"$IMG_PATH/spark/bindings/python/Dockerfile"}
    -  local RDOCKERFILE=${RDOCKERFILE:-"$IMG_PATH/spark/bindings/R/Dockerfile"}
    +  local 
BASEDOCKERFILE=${BASEDOCKERFILE:-"$IMG_PATH/main/dockerfiles/spark/Dockerfile"}
    +  local 
PYDOCKERFILE=${PYDOCKERFILE:-"$IMG_PATH/main/dockerfiles/spark/bindings/python/Dockerfile"}
    +  local 
KDOCKERFILE=${KDOCKERFILE:-"$IMG_PATH/test/dockerfiles/spark/kerberos/Dockerfile"}
    --- End diff --
    
    At the minimum, it might confuse people because people who check this 
script will possibly see there's now a `spark-keberos`  image without even 
noticing that the Dockerfile is meant for **tests**.  Given that we have the 
`resource-manager/kubernetes/integration-tests` directory, why not put 
test-specific Dockerfile and scripts under that directory?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to