Skip to content

Conversation

@LuciferYang
Copy link
Contributor

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

@LuciferYang
Copy link
Contributor Author

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

@LuciferYang LuciferYang changed the title [SPARK-40415][BUILD] Add explicit Maven dependency for okio [SPARK-40415][BUILD][K8S] Add explicit Maven dependency for okio Sep 13, 2022
@srowen
Copy link
Member

srowen commented Sep 13, 2022

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.

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Sep 13, 2022

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.

@LuciferYang
Copy link
Contributor Author

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?

@srowen
Copy link
Member

srowen commented Sep 13, 2022

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

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Sep 13, 2022

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

Got it, if there is no actual risk, agree to not to make this change

@dongjoon-hyun
Copy link
Member

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

@LuciferYang
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants