You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "MeihanLi (via GitHub)" <gi...@apache.org> on 2023/08/04 23:27:26 UTC
[GitHub] [pinot] MeihanLi opened a new pull request, #11273: Add Server API to get tenant pools
MeihanLi opened a new pull request, #11273:
URL: https://github.com/apache/pinot/pull/11273
Add a new Server API GET /instance/pools in InstanceResource to get server pools.
`feature`
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] MeihanLi commented on pull request #11273: Add Server API to get tenant pools
Posted by "MeihanLi (via GitHub)" <gi...@apache.org>.
MeihanLi commented on PR #11273:
URL: https://github.com/apache/pinot/pull/11273#issuecomment-1666285206
cc: @chenboat can you help review it? 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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] MeihanLi commented on a diff in pull request #11273: Add Server API to get tenant pools
Posted by "MeihanLi (via GitHub)" <gi...@apache.org>.
MeihanLi commented on code in PR #11273:
URL: https://github.com/apache/pinot/pull/11273#discussion_r1286277585
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/InstanceResource.java:
##########
@@ -73,4 +75,17 @@ public List<String> getInstanceTags() {
}
return Collections.emptyList();
}
+
+ @GET
+ @Path("pools")
+ @ApiOperation(value = "Tenant pools for current instance")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
+ })
+ @Produces(MediaType.APPLICATION_JSON)
+ public Map<String, String> getInstancePools() {
+ InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, _instanceId);
Review Comment:
Thanks Ankit, addressed
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat merged pull request #11273: Add Server API to get tenant pools
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat merged PR #11273:
URL: https://github.com/apache/pinot/pull/11273
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a diff in pull request #11273: Add Server API to get tenant pools
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #11273:
URL: https://github.com/apache/pinot/pull/11273#discussion_r1291549364
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/InstanceResource.java:
##########
@@ -73,4 +75,20 @@ public List<String> getInstanceTags() {
}
return Collections.emptyList();
}
+
+ @GET
+ @Path("pools")
+ @ApiOperation(value = "Tenant pools for current instance")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
+ })
Review Comment:
1) can you add the javadoc to explain return result types? i.e.,The summary in the PR looks good.
2) Can you add a Pinot doc reference to the definition of pool in Pinot?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ankitsultana commented on a diff in pull request #11273: Add Server API to get tenant pools
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #11273:
URL: https://github.com/apache/pinot/pull/11273#discussion_r1286207981
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/InstanceResource.java:
##########
@@ -73,4 +75,17 @@ public List<String> getInstanceTags() {
}
return Collections.emptyList();
}
+
+ @GET
+ @Path("pools")
+ @ApiOperation(value = "Tenant pools for current instance")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
+ })
+ @Produces(MediaType.APPLICATION_JSON)
+ public Map<String, String> getInstancePools() {
+ InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, _instanceId);
Review Comment:
need null check for instanceConfig (similar to getInstanceTags)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #11273: Add Server API to get tenant pools
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11273:
URL: https://github.com/apache/pinot/pull/11273#issuecomment-1666301862
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11273?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11273](https://app.codecov.io/gh/apache/pinot/pull/11273?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e390e53) into [master](https://app.codecov.io/gh/apache/pinot/commit/0984168988551b97c5a2884f3d2588e75975ff3b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0984168) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11273 +/- ##
=========================================
Coverage 0.11% 0.11%
=========================================
Files 2229 2155 -74
Lines 119963 116809 -3154
Branches 18183 17738 -445
=========================================
Hits 137 137
+ Misses 119806 116652 -3154
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `?` | |
| integration1temurin20 | `?` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `?` | |
| unittests1temurin20 | `?` | |
| unittests2temurin11 | `?` | |
| unittests2temurin17 | `0.11% <0.00%> (ø)` | |
| unittests2temurin20 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files Changed](https://app.codecov.io/gh/apache/pinot/pull/11273?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...e/pinot/server/api/resources/InstanceResource.java](https://app.codecov.io/gh/apache/pinot/pull/11273?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9JbnN0YW5jZVJlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [76 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11273/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org