You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/09 17:27:13 UTC
[GitHub] [pinot] zhtaoxiang opened a new pull request, #9359: Improve segment name check in metadata push
zhtaoxiang opened a new pull request, #9359:
URL: https://github.com/apache/pinot/pull/9359
Improve segment name check in metadata push:
1/ we can now push segments stored in Pinot deep store (owned by other tables)
2/ validate the segment name
--
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] Jackie-Jiang commented on a diff in pull request #9359: Improve segment name check in metadata push
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9359:
URL: https://github.com/apache/pinot/pull/9359#discussion_r967435756
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/name/SegmentNameGenerator.java:
##########
@@ -37,7 +37,7 @@ public interface SegmentNameGenerator extends Serializable {
* @param segmentName provide segment name
* @return true if segmentName is valid.
*/
- default boolean isValidSegmentName(String segmentName) {
+ static boolean isValidSegmentName(String segmentName) {
Review Comment:
Let's create a `SegmentNameUtils` class and move this method there
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/name/SegmentNameGenerator.java:
##########
@@ -37,7 +37,7 @@ public interface SegmentNameGenerator extends Serializable {
* @param segmentName provide segment name
* @return true if segmentName is valid.
*/
- default boolean isValidSegmentName(String segmentName) {
+ static boolean isValidSegmentName(String segmentName) {
Review Comment:
Consider changing it to `void validateSegmentName(String segmentName)` and throw `IllegalArgumentException` to simplify the caller code.
--
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] zhtaoxiang commented on a diff in pull request #9359: Improve segment name check in metadata push
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #9359:
URL: https://github.com/apache/pinot/pull/9359#discussion_r967509098
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/name/SegmentNameGenerator.java:
##########
@@ -37,7 +37,7 @@ public interface SegmentNameGenerator extends Serializable {
* @param segmentName provide segment name
* @return true if segmentName is valid.
*/
- default boolean isValidSegmentName(String segmentName) {
+ static boolean isValidSegmentName(String segmentName) {
Review Comment:
thanks for the feedback, updated the PR.
--
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] KKcorps merged pull request #9359: Improve segment name check in metadata push
Posted by GitBox <gi...@apache.org>.
KKcorps merged PR #9359:
URL: https://github.com/apache/pinot/pull/9359
--
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 #9359: Improve segment name check in metadata push
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9359:
URL: https://github.com/apache/pinot/pull/9359#issuecomment-1242561690
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9359?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 [#9359](https://codecov.io/gh/apache/pinot/pull/9359?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d7970f) into [master](https://codecov.io/gh/apache/pinot/commit/c8a114db4835ce88654708c26a535f9779dfab15?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c8a114d) will **decrease** coverage by `54.47%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #9359 +/- ##
=============================================
- Coverage 69.75% 15.28% -54.48%
+ Complexity 4707 170 -4537
=============================================
Files 1884 1832 -52
Lines 100273 98018 -2255
Branches 15252 14995 -257
=============================================
- Hits 69942 14978 -54964
- Misses 25380 81907 +56527
+ Partials 4951 1133 -3818
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.28% <0.00%> (-0.01%)` | :arrow_down: |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9359?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/SegmentPushUtils.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9TZWdtZW50UHVzaFV0aWxzLmphdmE=) | `0.00% <0.00%> (-12.50%)` | :arrow_down: |
| [...nt/spi/creator/name/FixedSegmentNameGenerator.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2NyZWF0b3IvbmFtZS9GaXhlZFNlZ21lbnROYW1lR2VuZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| [...eator/name/NormalizedDateSegmentNameGenerator.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2NyZWF0b3IvbmFtZS9Ob3JtYWxpemVkRGF0ZVNlZ21lbnROYW1lR2VuZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-83.06%)` | :arrow_down: |
| [...segment/spi/creator/name/SegmentNameGenerator.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2NyZWF0b3IvbmFtZS9TZWdtZW50TmFtZUdlbmVyYXRvci5qYXZh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...not/segment/spi/creator/name/SegmentNameUtils.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2NyZWF0b3IvbmFtZS9TZWdtZW50TmFtZVV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...t/spi/creator/name/SimpleSegmentNameGenerator.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2NyZWF0b3IvbmFtZS9TaW1wbGVTZWdtZW50TmFtZUdlbmVyYXRvci5qYXZh) | `0.00% <0.00%> (-84.22%)` | :arrow_down: |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1423 more](https://codecov.io/gh/apache/pinot/pull/9359/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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: 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