You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/09/13 15:26:46 UTC

[GitHub] [spark] LuciferYang opened a new pull request, #37867: [SPARK-40415][BUILD] Add explicit Maven dependency for okio

LuciferYang opened a new pull request, #37867:
URL: https://github.com/apache/spark/pull/37867

   ### What changes were proposed in this pull request?
   There are two places in Spark that depend on okio
   
   ```
   [INFO] +- org.seleniumhq.selenium:selenium-java:jar:3.141.59:test
   ...
   [INFO] |  +- com.squareup.okhttp3:okhttp:jar:3.11.0:test
   [INFO] |  \- com.squareup.okio:okio:jar:1.14.0:test
   ```
   
   and 
   
   ```
   [INFO] +- io.fabric8:kubernetes-client:jar:5.12.3:compile
   ....
   [INFO] |  +- com.squareup.okhttp3:okhttp:jar:3.12.12:compile
   [INFO] |  |  \- com.squareup.okio:okio:jar:1.15.0:compile
   ```
   
   But `Spark Project Assembly` chose `com.squareup.okio:okio:jar:1.14.0` instead of `com.squareup.okio:okio:jar:1.15.0` as compile scope dependency
   
   ```
   [INFO] +- org.apache.spark:spark-kubernetes_2.12:jar:3.4.0-SNAPSHOT:compile
   [INFO] |  +- io.fabric8:kubernetes-client:jar:5.12.3:compile
   [INFO] |  |  +- com.squareup.okhttp3:okhttp:jar:3.12.12:compile
   ...
   [INFO] +- org.scalatestplus:selenium-3-141_2.12:jar:3.2.10.0:test
   [INFO] |  |  +- org.apache.commons:commons-exec:jar:1.3:test
   [INFO] |  |  \- com.squareup.okio:okio:jar:1.14.0:compile
   ```
   
   and the okio version in spark-deps file is also 1.14.0, this seems to be an incorrect behavior, so this pr explicitly adds the dependency definition of `com.squareup.okio:okio` to clarify that the 1.15.0 used by `kubernetes-client` is the compile scope dependency.
   
   
   
   ### Why are the changes needed?
   Should use okio 1.15.0 in spark-deps files.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Pass GitHub Actions


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] srowen commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245588571

   Hm, does it cause a problem? Maven tends to adopt "nearest first" semantics, which can be right or wrong, but I don't think we'd explicitly manage it if it's working.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245612332

   > That's right, IIRC. sbt tends to favor newer versions, not nearest.
   
   Got it, iff there is no actual risk, agree not to make this change
   
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245598203

   I found this issue when I was resolving another jira(upgrade org.scalatestplus:selenium). I think we need @dongjoon-hyun or @Yikun help to check whether there is a risk in using a higher version of `kubernetes-client` and low version of `okio`,  they should know more about K8S.
   
   
   
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245631337

   > According to K8s IT, it seems that there is no evidence of failures, @LuciferYang . In that case, I also prefer to close this.
   
   Got it, thanks @srowen @dongjoon-hyun 


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang commented on pull request #37867: [SPARK-40415][BUILD] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245583851

   Explicit definition of okio 1.15.0 or okhttp3 3.12.12 can both achieve the goal, which is better? 
   
   cc @dongjoon-hyun @Yikun @srowen for help


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] dongjoon-hyun commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245630128

   According to K8s IT, it seems that there is no evidence of failures, @LuciferYang .
   In that case, I also prefer to close this.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245606959

   > Maven tends to adopt "nearest first" semantics
   
   Do you mean this may be because okio 1.15.0 is a level 4 dependency and okio 1.14.0 is a level 3 dependency, so 1.14.0 is used as the final compile scope dependent version?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] srowen commented on pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37867:
URL: https://github.com/apache/spark/pull/37867#issuecomment-1245609120

   That's right, IIRC. sbt tends to favor newer versions, not nearest.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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


[GitHub] [spark] LuciferYang closed pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio

Posted by GitBox <gi...@apache.org>.
LuciferYang closed pull request #37867: [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio
URL: https://github.com/apache/spark/pull/37867


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


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