-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-7505: [Java] Remove Netty dependency for ArrowBuf #6131
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
Conversation
|
@jacques-n Would you please take a look? |
|
I checked in the int64 address space change so this PR and probably a few other might need to be rebased. |
Sure. Thanks for your kind reminder. |
ec1715e to
156852f
Compare
|
@jacques-n or @rymurr do you have bandwidth to at least do a first pass review? |
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.
Should we try to mimic exactly how it is done in PlatformDependent0?
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 mean do we need all the checks that done there?
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.
Thanks a lot for your comments. I will try to mimic the logic in PlatformDependent0.
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.
@siddharthteotia I have revised the code to mimic PlatformDependent implementation. Would you please take another look?
784b050 to
c486750
Compare
|
@liyafan82 , the changes look good to me. Looks like there is a build failure |
@siddharthteotia Thanks a lot for your effort. It seems all my PRs are experiencing this failure. I will do a rebase when the problem is resolved. |
c486750 to
97b84ae
Compare
|
@siddharthteotia can this be merged, i think the integration test is unrelated? |
|
Thanks, @liyafan82 . Merging this as the integration test failure seems to be unrelated. @liyafan82 , it would be great if you can keep an eye on the build and make sure the build on master is clean after this PR is merged (just in case the integration test failure happens to be caused by this change) |
@siddharthteotia Thanks a lot for your effort. I will keep an eye on the master branch. |
* [ARROW-7505][Java] Remove Netty dependency for ArrowBuf * [ARROW-7505][Java] Mimic PlatformDependent implementation
This is part of the first step of issue ARROW-4526.
In this step, we remove netty dependency for ArrowBuf, BufferAllocator and ReferenceManager.
In this issue, we remove the dependency for ArrowBuf.
The task for BufferAllocator and ReferenceManager will not start until ARROW-7329 is finished.