-
Notifications
You must be signed in to change notification settings - Fork 3k
Spark4.0 rc5 integration with Iceberg #13022
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,6 +120,9 @@ allprojects { | |
| repositories { | ||
| mavenCentral() | ||
| mavenLocal() | ||
| maven { | ||
| url "https://repository.apache.org/content/repositories/orgapachespark-1484/" | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we planning to merge this PR with RC5? I think it is not a good practice to depend on RC on main branch. Why don't we continue the development in the separate branch till the official release available?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@ajantha-bhat It's true that we'd have a dependency on an RC on main but there's a benefit to being able to develop on top of it for any new integrations (e.g. for V3 for example) while the RC is still going because we don't expect those dependent features to fundamentally change between RC and release. Keep in mind the defaultSparkVersion and any infra like benchmarking will still default to 3.5 until the official release. The main challenge is that continuing development in the separate branch means that an individual needs to keep rebasing and evaluating if a change needs to be kept in sync with 4.0 with any intermediate changes to 3.4/3.5. Merging the initial means that every subsequent change to Iceberg-Spark puts it on the author of that change to keep it in sync, which is more narrow and that author will have a lot more context. Combining that with the previous point about new integrations simply means we can safely and reasonably iterate on 4.0 integration until the official release rather than an individual just wait for all of that while rebasing/keeping in sync. That feels worthwhile to me compared to the awkwardness of having a RC dependency in main in the short term
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Thanks for the explanation. I am neutral about this. If community agrees to depend on RC, it works for me. Just make sure, it has more visibility.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, I went ahead and added a few folks for reviews. Thanks!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, as RC5 is getting closer to Spark 4.0 release (only blockers are getting in now), I think it makes sense to work on Iceberg in parallel as we dont anticipate any significant change bumping to the final RC |
||
| } | ||
| } | ||
|
|
||
|
|
||
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.
what's the reason for removing JDK 11 from this? we're releasing JDK 11 jars so I think we should keep testing with JDK 11
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.
https://github.com/apache/iceberg/pull/12494/files#r2045335206