You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "snleee (via GitHub)" <gi...@apache.org> on 2024/02/03 18:13:18 UTC
[PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
snleee opened a new pull request, #12360:
URL: https://github.com/apache/pinot/pull/12360
- Current server side API throws the error if the snapshot file is not availble. In this case, we should skip instead of throwing the error.
- Improve the API doc for validDocIds related APIs
--
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
Re: [PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12360:
URL: https://github.com/apache/pinot/pull/12360#issuecomment-1925428907
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12360?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `6 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`ab525ee`)](https://app.codecov.io/gh/apache/pinot/commit/ab525ee9296fec122135fb5f877c45fa37f9c376?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.72% compared to head [(`5e17419`)](https://app.codecov.io/gh/apache/pinot/pull/12360?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 34.98%.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/pinot/server/api/resources/TablesResource.java](https://app.codecov.io/gh/apache/pinot/pull/12360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12360 +/- ##
=============================================
- Coverage 61.72% 34.98% -26.74%
+ Complexity 207 6 -201
=============================================
Files 2426 2350 -76
Lines 132667 128920 -3747
Branches 20499 19941 -558
=============================================
- Hits 81888 45105 -36783
- Misses 44774 80592 +35818
+ Partials 6005 3223 -2782
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.84% <ø> (-14.83%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.87% <0.00%> (-26.72%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.96% <0.00%> (-26.76%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.85% <0.00%> (-26.69%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.98% <0.00%> (-26.74%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.90% <ø> (-14.82%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.90% <ø> (-0.02%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12360?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: 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
Re: [PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee merged PR #12360:
URL: https://github.com/apache/pinot/pull/12360
--
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
Re: [PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #12360:
URL: https://github.com/apache/pinot/pull/12360#discussion_r1477122121
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -961,7 +961,7 @@ public String getTableAggregateValidDocIdMetadata(
@ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
@ApiParam(value = "A list of segments", allowMultiple = true) @QueryParam("segmentNames")
List<String> segmentNames,
- @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE")
+ @ApiParam(value = "Valid doc id type (SNAPSHOT | IN_MEMORY | IN_MEMORY_WITH_DELETE)")
@QueryParam("validDocIdsType") String validDocIdsType) {
Review Comment:
changed to enum
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -961,7 +961,7 @@ public String getTableAggregateValidDocIdMetadata(
@ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
@ApiParam(value = "A list of segments", allowMultiple = true) @QueryParam("segmentNames")
List<String> segmentNames,
- @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE")
+ @ApiParam(value = "Valid doc id type (SNAPSHOT | IN_MEMORY | IN_MEMORY_WITH_DELETE)")
Review Comment:
changed the doc
--
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
Re: [PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #12360:
URL: https://github.com/apache/pinot/pull/12360#discussion_r1477122176
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -610,7 +614,7 @@ public String getValidDocIdMetadata(
public String getValidDocIdMetadata(
Review Comment:
changed to `validDocIdMetadata -> calidDocIdsMetadata`
--
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
Re: [PR] Skip instead of throwing error on 'getValidDocIdMetadata' [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12360:
URL: https://github.com/apache/pinot/pull/12360#discussion_r1477116983
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -961,7 +961,7 @@ public String getTableAggregateValidDocIdMetadata(
@ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
@ApiParam(value = "A list of segments", allowMultiple = true) @QueryParam("segmentNames")
List<String> segmentNames,
- @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE")
+ @ApiParam(value = "Valid doc id type (SNAPSHOT | IN_MEMORY | IN_MEMORY_WITH_DELETE)")
@QueryParam("validDocIdsType") String validDocIdsType) {
Review Comment:
Can we directly take enum here?
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -610,7 +614,7 @@ public String getValidDocIdMetadata(
public String getValidDocIdMetadata(
Review Comment:
Introduced in the previous PR. Can we change this to GET request? Maybe change the path to "/tables/{tableNameWithType}/validDocIdsMetadata"
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -961,7 +961,7 @@ public String getTableAggregateValidDocIdMetadata(
@ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr,
@ApiParam(value = "A list of segments", allowMultiple = true) @QueryParam("segmentNames")
List<String> segmentNames,
- @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE")
+ @ApiParam(value = "Valid doc id type (SNAPSHOT | IN_MEMORY | IN_MEMORY_WITH_DELETE)")
Review Comment:
```suggestion
@ApiParam(value = "Valid doc ids type (SNAPSHOT | IN_MEMORY | IN_MEMORY_WITH_DELETE)")
```
--
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