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

[KYUUBI #6293] Upgrade Arrow from 12.0.0 to 15.0.2 #6304

Closed
wants to merge 6 commits into from

Conversation

dupen01
Copy link
Contributor

@dupen01 dupen01 commented Apr 13, 2024

🔍 Description

Issue References 🔗

This pull request fixes #6293

Describe Your Solution 🔧

Here are my main modifications:

  1. In the project POM file, I have updated the Apache Arrow version from 12.0.0 to 15.0.2.
  2. After executing build/dependency.sh --replace, the dev/dependencyList file was automatically updated.
  3. I noticed the addition of the "Eclipse Collections" dependency and checked its open-source license. Following the existing format, I have accordingly modified the NOTICE-binary and LICENSE-binary files.

However, I remain uncertain whether my changes to the LICENSE/NOTICE sections adhere to the community's standards. I kindly request guidance from the community regarding this matter.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@github-actions github-actions bot added kind:infra license, community building, project builds, asf infra related, etc. kind:build labels Apr 13, 2024
LICENSE-binary Outdated

Eclipse Public License (EPL) 1.0
--------------------------------
org.eclipse.collections:eclipse-collections-api
Copy link
Member

Choose a reason for hiding this comment

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

for dual Licensed dependency, we only need to choose one. for this case, let's choose EDL(listed in category A) https://apache.org/legal/resolved.html

NOTICE-binary Outdated
* licenses/LICENSE-EDL-1.0.txt (Eclipse Distribution License 1.0 (BSD))
* licenses/LICENSE-EPL-1.0.txt (Eclipse Public License 1.0)
* HOMEPAGE:
* https://eclipse.dev/collections/index.html
Copy link
Member

Choose a reason for hiding this comment

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

nit: new line at end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, about the LICENSE information of third-party dependencies mentioned within the NOTICE-binary file, I have two points of confusion:

  1. For instance, for 'lz4-java', its LICENSE information points to 'license/LICENSE.lz4-java.txt', it looks like a file path, but this file is not actually found in the project directory. I am unsure whether this is an error or if there is some other meaning behind it.

  2. I have noticed that there are two folders related to licenses at the root of the project: licenses and licenses-binary, and I am unclear about what types of license files each of these folders should contain.

I would appreciate clarification on how the community differentiates and considers these two directories.

@pan3793
Copy link
Member

pan3793 commented Apr 13, 2024

CI still running, I think it will fail eventually because kyuubi-hive-jdbc-shaded should be updated too

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.41%. Comparing base (9086cfe) to head (62968d2).

❗ Current head 62968d2 differs from pull request most recent head e858a1c. Consider uploading reports for the commit e858a1c to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6304      +/-   ##
============================================
- Coverage     58.47%   58.41%   -0.06%     
  Complexity       24       24              
============================================
  Files           652      652              
  Lines         39645    39645              
  Branches       5454     5454              
============================================
- Hits          23181    23159      -22     
- Misses        13984    13994      +10     
- Partials       2480     2492      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dupen01
Copy link
Contributor Author

dupen01 commented Apr 13, 2024

CI still running, I think it will fail eventually because kyuubi-hive-jdbc-shaded should be updated too

I will update it in the next commit

@@ -0,0 +1,70 @@
Eclipse Public License - v 1.0
Copy link
Member

Choose a reason for hiding this comment

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

delete this file, because we don't include EPL 1.0 third-party jars in the binary distribution

@@ -224,3 +224,9 @@ MIT License
-----------
org.slf4j:slf4j-api
org.slf4j:jcl-over-slf4j

Copy link
Member

Choose a reason for hiding this comment

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

we shaded arrow into kyuubi-hive-jdbc-shaded.jar, now the transitive deps of arrow changed, the kyuubi-hive-jdbc-shaded/pom.xml's shading rules need to be updated

Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

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

LGTM if CI pass

@pan3793 pan3793 closed this in 3133b59 Apr 15, 2024
@pan3793 pan3793 added this to the v1.10.0 milestone Apr 15, 2024
@pan3793
Copy link
Member

pan3793 commented Apr 15, 2024

Thanks, merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:build kind:infra license, community building, project builds, asf infra related, etc. module:ctl module:hive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK][EASY] Upgrade Arrow from 12.0.0 to 15.0.2
3 participants