-
Notifications
You must be signed in to change notification settings - Fork 28k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-48251][BUILD] Disable maven local cache
on GA's step MIMA test
#46551
base: master
Are you sure you want to change the base?
Conversation
project/SparkBuild.scala
Outdated
Resolver.file("ivyLocal", file(Path.userHome.absolutePath + "/.ivy2/local"))(Resolver.ivyStylePatterns) | ||
), | ||
DefaultMavenRepository) ++ | ||
{ if (sys.env.contains("SKIP_LOCAL_M2")) Nil else Seq(Resolver.mavenLocal) } :+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... so, even if the value of SKIP_LOCAL_M2
is "", Resolver.mavenLocal
will be disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the conditions more stricter.
Updated.
project/SparkBuild.scala
Outdated
@@ -257,6 +257,7 @@ object SparkBuild extends PomBuild { | |||
|
|||
val noLintOnCompile = sys.env.contains("NOLINT_ON_COMPILE") && | |||
!sys.env.get("NOLINT_ON_COMPILE").contains("false") | |||
lazy val skipLocalM2 = sys.env.getOrElse("SKIP_LOCAL_M2", "false").toBoolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always need to check this status, right? So I don't think it needs to be marked as lazy
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully understand the history and the reason of this proposal, @panbingkun and @LuciferYang . And, thank you for trying this.
I'm still reluctant of using SKIP_LOCAL_M2
because I'm afraid that this will bite us laster in some local and downstream CI environments and eventually we might need to add SKIP_LOCAL_M2
always for all dev environments as a workaround of SBT bug. As a result, it causes a slowdown in whole CIs (instead of MIMA test only).
To be clear, I'll leave this to your decision.
Also, cc @cloud-fan , @HyukjinKwon , @mridulm , @yaooqinn too.
If this might increase the $ cost of ASF INFRA, let's put it on hold until the upstream bug is fixed. I second @dongjoon-hyun's concern. |
@panbingkun How much performance gap will there be if we completely abandon using |
Let me test it. |
@LuciferYang
|
also cc @dongjoon-hyun @yaooqinn |
What changes were proposed in this pull request?
The pr aims to
disable
maven local cache
on GA's stepMIMA test
.Why are the changes needed?
This is the
pre-pr
of upgradingsbt
from1.9.3
to ahigher version
of sbt.See:
sbt/sbt#7506 (comment)
coursier/coursier#2942
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GA.
Was this patch authored or co-authored using generative AI tooling?
No.