You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "kaijchen (via GitHub)" <gi...@apache.org> on 2023/02/01 07:58:14 UTC
[GitHub] [incubator-uniffle] kaijchen opened a new pull request, #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
kaijchen opened a new pull request, #536:
URL: https://github.com/apache/incubator-uniffle/pull/536
### What changes were proposed in this pull request?
Remove UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD from `spotbugs-exclude.xml` and fix violations.
### Why are the changes needed?
Sub-task of #532 - Fix [UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD][1].
[1]: https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#uwf-unwritten-public-or-protected-field-uwf-unwritten-public-or-protected-field
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#issuecomment-1413302332
Thanks @advancedxy for the review.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#issuecomment-1413079193
The code is fine with me. But I'm not sure why we treat `GRPC_OPEN` and `GRPC_TOTAL` differently.
See @zuston if you have more input.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#issuecomment-1413089012
> The code is fine with me. But I'm not sure why we treat `GRPC_OPEN` and `GRPC_TOTAL` differently. See @zuston if you have more input.
Nice catch.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#issuecomment-1411683958
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#536](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44b1e5c) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/721b22cf0a02c0d3fdb65851aa1edd8271dbe138?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (721b22c) will **decrease** coverage by `0.04%`.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## master #536 +/- ##
============================================
- Coverage 60.23% 60.19% -0.04%
+ Complexity 1785 1784 -1
============================================
Files 205 205
Lines 11557 11561 +4
Branches 1042 1042
============================================
- Hits 6961 6959 -2
- Misses 4189 4194 +5
- Partials 407 408 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/uniffle/common/metrics/GRPCMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9tZXRyaWNzL0dSUENNZXRyaWNzLmphdmE=) | `43.83% <0.00%> (-2.55%)` | :arrow_down: |
| [...fle/coordinator/metric/CoordinatorGrpcMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29vcmRpbmF0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3VuaWZmbGUvY29vcmRpbmF0b3IvbWV0cmljL0Nvb3JkaW5hdG9yR3JwY01ldHJpY3MuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...pache/uniffle/server/ShuffleServerGrpcMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyR3JwY01ldHJpY3MuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...ategy/storage/AppBalanceSelectStorageStrategy.java](https://codecov.io/gh/apache/incubator-uniffle/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29vcmRpbmF0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3VuaWZmbGUvY29vcmRpbmF0b3Ivc3RyYXRlZ3kvc3RvcmFnZS9BcHBCYWxhbmNlU2VsZWN0U3RvcmFnZVN0cmF0ZWd5LmphdmE=) | `72.00% <0.00%> (-4.00%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen merged pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen merged PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#discussion_r1093261940
##########
coordinator/src/main/java/org/apache/uniffle/coordinator/metric/CoordinatorGrpcMetrics.java:
##########
@@ -35,8 +35,8 @@ public class CoordinatorGrpcMetrics extends GRPCMetrics {
@Override
public void registerMetrics() {
- gaugeGrpcOpen = metricsManager.addGauge(GRPC_OPEN);
- counterGrpcTotal = metricsManager.addCounter(GRPC_TOTAL);
+ setGaugeGrpcOpen(metricsManager.addGauge(GRPC_OPEN));
Review Comment:
These two lines are same with ShuffleServer. isn't better to just initialize these metrics in parent class?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #536: [SpotBugs] Fix UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #536:
URL: https://github.com/apache/incubator-uniffle/pull/536#issuecomment-1413081733
> The code is fine with me. But I'm not sure why we treat `GRPC_OPEN` and `GRPC_TOTAL` differently.
It's written by different authors. Maybe because `GRPC_OPEN` and `GRPC_TOTAL` are accessed more frequently.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org