Skip to content

[Don't Merge] upgrade sbt 1.9.8 - #44516

Closed
panbingkun wants to merge 15 commits into
apache:masterfrom
panbingkun:sbt_1_9_8
Closed

panbingkun wants to merge 15 commits into
apache:masterfrom
panbingkun:sbt_1_9_8

Conversation

@panbingkun

@panbingkun panbingkun commented Dec 28, 2023 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The pr aims to upgrade sbt from 1.9.3 to 1.9.8.

Why are the changes needed?

After version sbt 1.9.3, if there are corrupt jars under the local (~/. m2), some workflows will fail to compile. Based on this, we added an environment variable (SKIP_LOCAL_M2) in SparkBuild to control whether to use the Maven Local Resolver mechanism.

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.

@panbingkun panbingkun changed the title Upgrad sbt 1.9.8 [Don't merge & review] Upgrad sbt 1.9.8 Dec 28, 2023
@panbingkun panbingkun changed the title [Don't merge & review] Upgrad sbt 1.9.8 [Don't merge & review] upgrade sbt 1.9.8 Dec 28, 2023
Comment thread project/SparkBuild.scala Outdated
}
)

lazy val cachePolicies = if (sys.env.contains("SKIP_LOCAL_M2")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is necessary, this work should be completed in a separate PR first.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, Let me try this solution first 😄

@panbingkun

panbingkun commented Jan 2, 2024 •

Copy link
Copy Markdown
Contributor Author

I changed my approach and modified some of the logic in SparkBuild by using an environment variable SKIP_ M2_ LOCAL is used to determine whether we skip the local maven cache (as some of our GA's corrupted jars in local maven cache(~/.m2/...) have caused some of our workflows to fail).

Friendly ping @dongjoon-hyun,
We would like to hear your suggestion. Is this current approach relatively feasible?

Comment thread project/SparkBuild.scala
Resolver.file("ivyLocal", file(Path.userHome.absolutePath + "/.ivy2/local"))(Resolver.ivyStylePatterns)
),
DefaultMavenRepository) ++
{ if (sys.env.contains("SKIP_LOCAL_M2")) Nil else Seq(Resolver.mavenLocal) } :+

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main modification points.

@panbingkun panbingkun changed the title [Don't merge & review] upgrade sbt 1.9.8 [Don't Merge] upgrade sbt 1.9.8 Jan 2, 2024
PYSPARK_DRIVER_PYTHON: python3.9
PYSPARK_PYTHON: python3.9
GITHUB_PREV_SHA: ${{ github.event.before }}
SKIP_LOCAL_M2: true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only for Linters, licenses, dependencies and documentation generation

@panbingkun

Copy link
Copy Markdown
Contributor Author

PTAL when you have some time, and Happy New Year 😄. @dongjoon-hyun

@dongjoon-hyun

Copy link
Copy Markdown
Member

Thank you for your deep investigation, @panbingkun . Happy New Year.
For me, this approach is still a little reluctant because depending on skipping the local M2 is easily broken at various user environments.
Although we can make CI works here, it will bite us back harder when something is broken. It's because more difficult to debug.

@dongjoon-hyun

Copy link
Copy Markdown
Member

If SBT does not provide any improvement, we may need to stick to the old SBT version.

@panbingkun

Copy link
Copy Markdown
Contributor Author

If SBT does not provide any improvement, we may need to stick to the old SBT version.

Okay, let's continue to observe the next version.

@github-actions

Copy link
Copy Markdown

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions Bot added the Stale label Apr 14, 2024
@panbingkun

Copy link
Copy Markdown
Contributor Author

@dongjoon-hyun

has an offline discussion with @LuciferYang, another approach maybe:

Should we ignore Maven repo cache for Mima tasks in both GA and developer environments?
so make it not easily broken at various user environments.

This bug in coursier, it is estimated that it will not be fixed for a while: coursier/coursier#2942 (comment)

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