You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "PorterZhang2021 (via GitHub)" <gi...@apache.org> on 2024/04/15 10:51:06 UTC
[PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
PorterZhang2021 opened a new pull request, #6310:
URL: https://github.com/apache/kyuubi/pull/6310
# :mag: Description
## Issue References ๐
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #6294
## Describe Your Solution ๐ง
1. Remove the dependency on `io.netty`from `pom.xml` at root
2. Modify the `pom.xml` of `kyuubi-server`
3. Run `/ Build/dependency. sh -- replace`
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] 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 :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist ๐
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Simplify Netty and gRPC dependency management [kyuubi]
Posted by "PorterZhang2021 (via GitHub)" <gi...@apache.org>.
PorterZhang2021 commented on code in PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#discussion_r1568549681
##########
pom.xml:
##########
@@ -774,74 +774,6 @@
</exclusions>
</dependency>
- <dependency>
- <groupId>io.netty</groupId>
- <artifactId>netty-all</artifactId>
- <version>${netty.version}</version>
- <exclusions>
- <exclusion>
Review Comment:
ok, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Simplify Netty and gRPC dependency management [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #6310: [KYUUBI #6294] Simplify Netty and gRPC dependency management
URL: https://github.com/apache/kyuubi/pull/6310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#issuecomment-2060248542
seems a little bit more complex than I originally thought, I will take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "PorterZhang2021 (via GitHub)" <gi...@apache.org>.
PorterZhang2021 commented on PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#issuecomment-2056555255
Hello @pan3793, Mr.Pan, I have attempted to remove the content of `io.netty`and have also tried packaging and testing again, but I have not actually conducted startup testing.
For other unused jars, after using `mvn dependency: analyze`, other pom.xml showed `unused declared dependencies found`, as mentioned below:
Because I am not sure about the impact of deletion and do not know how to test the impact, I did not delete it
If you have more detailed guidance, I may give it a try, thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#discussion_r1568359415
##########
pom.xml:
##########
@@ -774,74 +774,6 @@
</exclusions>
</dependency>
- <dependency>
- <groupId>io.netty</groupId>
- <artifactId>netty-all</artifactId>
- <version>${netty.version}</version>
- <exclusions>
- <exclusion>
Review Comment:
I just noticed I was wrong on this assumption, apologize for this comment.
The netty jars come from `grpc-netty`, `arrow-memory-netty`, use `netty-bom` and `grpc-bom` to simplify the dependency management.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "PorterZhang2021 (via GitHub)" <gi...@apache.org>.
PorterZhang2021 commented on code in PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#discussion_r1565636428
##########
pom.xml:
##########
@@ -774,74 +774,6 @@
</exclusions>
</dependency>
- <dependency>
- <groupId>io.netty</groupId>
- <artifactId>netty-all</artifactId>
- <version>${netty.version}</version>
- <exclusions>
- <exclusion>
Review Comment:
emmm, I know, I will try it again. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#discussion_r1565615586
##########
pom.xml:
##########
@@ -774,74 +774,6 @@
</exclusions>
</dependency>
- <dependency>
- <groupId>io.netty</groupId>
- <artifactId>netty-all</artifactId>
- <version>${netty.version}</version>
- <exclusions>
- <exclusion>
Review Comment:
actually, my vision is to extend this exclusion list instead of declaring everything explicitly. for example, `netty-codec-socks` does not used by Kyuubi, and we should add it into exclusion list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "PorterZhang2021 (via GitHub)" <gi...@apache.org>.
PorterZhang2021 commented on PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#issuecomment-2059165352
Hello, Mr.Pan @pan3793 , I try it again, and exclusion everyone, I removed all dependencies and tried to start it without any issues.
But for other related dependencies, I don't quite understand how to remove them. I did the following:
1. Use `mvn dependency: analyze` (using kyuubi_ha as an example)
2. Use `mvn dependency: tree`
3. open kyuubi_ha -> pom.xml, then delete `io.netty:netty-transport-native-epoll:jar:linux-aarch_64`, `io.netty:netty-transport-native-epoll:jar:linux-x86_64`
4. then repackages
I'm not sure if my example is accurate or not. If it's not, can you provide me with some relevant reference materials? I've searched a lot online but I still don't quite understand this aspect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Prune unused Netty libraries [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#issuecomment-2056982169
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 58.44%. Comparing base [(`9086cfe`)](https://app.codecov.io/gh/apache/kyuubi/commit/9086cfe054969320bc5a29ffbbdd11e2412abfff?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e0676ed`)](https://app.codecov.io/gh/apache/kyuubi/pull/6310?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 3 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6310 +/- ##
============================================
- Coverage 58.47% 58.44% -0.03%
Complexity 24 24
============================================
Files 652 653 +1
Lines 39645 39755 +110
Branches 5454 5466 +12
============================================
+ Hits 23181 23236 +55
- Misses 13984 14025 +41
- Partials 2480 2494 +14
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #6294] Simplify Netty and gRPC dependency management [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6310:
URL: https://github.com/apache/kyuubi/pull/6310#issuecomment-2060591828
Merged to master/1.9.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org