Skip to content
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

Updated Dotnet and Arcade #1197

Merged
merged 5 commits into from
Jan 14, 2025
Merged

Conversation

grazy27
Copy link
Contributor

@grazy27 grazy27 commented Jan 3, 2025

Changes:

  • Updated .Net6 -> .Net8
  • Updated .Net Framework 4.6.1 -> 4.8
  • Updated Nuget dependencies
  • Updated Arcade 1.0 -> 8. Used release 8.0 branch
  • Added a few git ignores and fixed a warning in the build pipeline

Affected Tickets:

@grazy27 grazy27 marked this pull request as ready for review January 3, 2025 17:09
.vscode/settings.json Outdated Show resolved Hide resolved
LICENSE Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we split this into another PR? Since we are not familiar with this, we may need to involve some people from legal department to review. Splitting them would be clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's part of Arcade validation pipeline, the same license is used all dotnet MIT projects

Arcade PR for license validation

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, let me check how to handle this.

@@ -151,7 +151,6 @@ extends:
- script: build.cmd -pack
-c $(buildConfiguration)
-ci
$(_OfficialBuildIdArgs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For pr pipeline - we don't define it
image

eng/AfterSolutionBuild.targets Outdated Show resolved Hide resolved
@grazy27
Copy link
Contributor Author

grazy27 commented Jan 7, 2025

Removed vscode config as a separate commit

Btw, there's .NET9 already available, but it's not LTS, so I still chose .NET8. Does my logic make sense to you?

@wudanzy wudanzy mentioned this pull request Jan 7, 2025
@wudanzy wudanzy requested a review from SparkSnail January 8, 2025 03:04
@wudanzy
Copy link
Collaborator

wudanzy commented Jan 8, 2025

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@grazy27
Copy link
Contributor Author

grazy27 commented Jan 8, 2025

@wudanzy build timed out, could you please share the logs where exactly did it stall?

@wudanzy
Copy link
Collaborator

wudanzy commented Jan 8, 2025

Maybe flaky, retried.

@wudanzy
Copy link
Collaborator

wudanzy commented Jan 9, 2025

Hi @grazy27, you have more experience than me on .NET. Do you know if the upgraded .NET 8 (or net48) runtime can run spark jobs compiled by .NET 6 (or net47)? Or do we need an integration test?

Copy link
Collaborator

@wudanzy wudanzy left a comment

Choose a reason for hiding this comment

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

LGTM from my sides, let @SparkSnail to have another look

@grazy27
Copy link
Contributor Author

grazy27 commented Jan 10, 2025

Hi @grazy27, you have more experience than me on .NET. Do you know if the upgraded .NET 8 (or net48) runtime can run spark jobs compiled by .NET 6 (or net47)? Or do we need an integration test?

In order to update Dotnet.Spark users will have to pull a new nuget, rebuild their project, and test that it's upgraded successfully. And if spark.worker is used - copypaste dlls to accessible location for it

Overall CLRs of 6 & 8, and 4.6 & 4.8 are compatible,
For framework, users might have to tinker with binding redirects in app.config.
For .NET8 - no changes needed, the only caveat is that there might be APIs that were deprecated or removed in .net8, so if .net6 lib references these APIs - there will be an error.

This situation can theoretically occur with spark.worker and UDF dlls, if users decide not to upgrade their project to 8:
image

```
You should see JARs created for the supported Spark versions:
* `microsoft-spark-2-3/target/microsoft-spark-2-3_2.11-<version>.jar`
* `microsoft-spark-2-4/target/microsoft-spark-2-4_2.11-<version>.jar`
* `microsoft-spark-3-0/target/microsoft-spark-3-0_2.12-<version>.jar`
* `microsoft-spark-3-0/target/microsoft-spark-3-5_2.12-<version>.jar`
Copy link
Collaborator

Choose a reason for hiding this comment

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

3-5

Copy link
Contributor Author

@grazy27 grazy27 Jan 14, 2025

Choose a reason for hiding this comment

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

Yup, I'll fix in the next PR

@wudanzy wudanzy merged commit f84b4fd into dotnet:main Jan 14, 2025
54 checks passed
@GeorgeS2019
Copy link

@SparkSnail @grazy27 @wudanzy @radical @shanselman what are next to do list?

@wudanzy
Copy link
Collaborator

wudanzy commented Jan 15, 2025

There is some discussion in #1162, and I personally want to remove 2.X support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants