You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Akanksha-kedia (via GitHub)" <gi...@apache.org> on 2025/10/24 16:44:39 UTC
[PR] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Akanksha-kedia opened a new pull request, #17072:
URL: https://github.com/apache/pinot/pull/17072
## Description
This change removes the deprecated `DEFAULT_SEGMENT_PUSH_TYPE` constant from `TableConfigBuilder.java` as indicated by the TODO comment. This is part of the ongoing effort to clean up deprecated code in preparation for a future major release.
## Changes Made
1. Removed the constant definition:
```java
private static final String DEFAULT_SEGMENT_PUSH_TYPE = "APPEND";
```
2. Updated the field initialization to use the literal value:
```java
@Deprecated
private String _segmentPushType = "APPEND";
```
3. Updated the `setSegmentPushType` method implementation to use the literal value:
```java
public TableConfigBuilder setSegmentPushType(String segmentPushType) {
if (REFRESH_SEGMENT_PUSH_TYPE.equalsIgnoreCase(segmentPushType)) {
_segmentPushType = REFRESH_SEGMENT_PUSH_TYPE;
} else {
_segmentPushType = "APPEND";
}
return this;
}
```
4. Removed the associated TODO comment since the task is now complete.
##
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-5087036875
cc @Jackie-Jiang @xiangfu0 @yashmayya — all tasks are done and the implementation is straightforward (inlining the literal values and removing the private constants). Would appreciate a final review and merge when you get a chance!
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r2467168174
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -54,7 +54,6 @@
public class TableConfigBuilder {
- private static final String DEFAULT_SEGMENT_PUSH_TYPE = "APPEND";
private static final String REFRESH_SEGMENT_PUSH_TYPE = "REFRESH";
Review Comment:
[nitpick] Consider defining a constant for 'APPEND' similar to `REFRESH_SEGMENT_PUSH_TYPE`. This would maintain consistency in the codebase and make future changes easier. For example: `private static final String APPEND_SEGMENT_PUSH_TYPE = \"APPEND\";`
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -76,9 +75,8 @@ public class TableConfigBuilder {
@Deprecated
private String _segmentPushFrequency;
- // TODO: Remove 'DEFAULT_SEGMENT_PUSH_TYPE' in the future major release.
@Deprecated
- private String _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
+ private String _segmentPushType = "APPEND";
Review Comment:
The hardcoded string literal 'APPEND' appears multiple times (lines 79 and 208). Consider extracting this into a constant to avoid magic strings and ensure consistency across all usages.
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-4661522566
@snleee @yashmayya @Jackie-Jiang — friendly ping on removing the deprecated DEFAULT_SEGMENT_PUSH_TYPE constant (has a TODO comment in the code). Happy to address any feedback.
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-4332150692
@xiangfu0 please 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: 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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r3448914510
##########
.github/workflows/japicmp-exclude.xml:
##########
@@ -16,18 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-#
-
-set -e
-
-MANIFEST_TAG="apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}"
-
-# Remove any existing local manifest so re-runs of a failed workflow don't fail
-# with "manifest already exists".
-docker manifest rm "${MANIFEST_TAG}" 2>/dev/null || true
-
-docker manifest create "${MANIFEST_TAG}" \
- --amend "apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}-amd64" \
- --amend "apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}-arm64"
-docker manifest push "${MANIFEST_TAG}"
+# Deprecated methods intentionally removed in this release
Review Comment:
Do not add this as this is an one time change
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-3449623945
@yashmayya
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r3535394585
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -76,9 +75,8 @@ public class TableConfigBuilder {
@Deprecated
private String _segmentPushFrequency;
- // TODO: Remove 'DEFAULT_SEGMENT_PUSH_TYPE' in the future major release.
@Deprecated
- private String _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
Review Comment:
Keeping `_segmentPushType`, `_segmentPushFrequency`, and their public getters/setters for now — removing public API in a minor version would fail the binary compatibility check (`METHOD_REMOVED` on `pinot-spi`).
The branch has been rebased cleanly to just remove the **private** constants `DEFAULT_SEGMENT_PUSH_TYPE` and `REFRESH_SEGMENT_PUSH_TYPE`, which were only used within `TableConfigBuilder` itself. The string literals are inlined in the deprecated `setSegmentPushType()` method body. Method removal can follow in the next major release.
##########
.github/workflows/japicmp-exclude.xml:
##########
@@ -16,18 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-#
-
-set -e
-
-MANIFEST_TAG="apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}"
-
-# Remove any existing local manifest so re-runs of a failed workflow don't fail
-# with "manifest already exists".
-docker manifest rm "${MANIFEST_TAG}" 2>/dev/null || true
-
-docker manifest create "${MANIFEST_TAG}" \
- --amend "apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}-amd64" \
- --amend "apachepinot/pinot-base-${BASE_IMAGE_TYPE}:${TAG}-arm64"
-docker manifest push "${MANIFEST_TAG}"
+# Deprecated methods intentionally removed in this release
Review Comment:
Agreed and done — the `japicmp-exclude.xml` file has been removed. The branch has been rebased from scratch; the only change is inlining the two private string constants.
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r3448132096
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -76,9 +75,8 @@ public class TableConfigBuilder {
@Deprecated
private String _segmentPushFrequency;
- // TODO: Remove 'DEFAULT_SEGMENT_PUSH_TYPE' in the future major release.
@Deprecated
- private String _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
Review Comment:
That was the original intent — this PR removes `DEFAULT_SEGMENT_PUSH_TYPE` (the constant itself) and its direct usages. The deprecated `_segmentPushType` / `_segmentPushFrequency` builder fields are being handled in a separate PR (#18472) to keep the blast radius small. Happy to consolidate into a single PR if you'd prefer.
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-3444055342
@snleee please review This change removes the deprecated DEFAULT_SEGMENT_PUSH_TYPE constant from TableConfigBuilder.java as indicated by the TODO comment. This is part of the ongoing effort to clean up deprecated code in preparation for a future major release.
@Jackie-Jiang @yashmayya
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-3446029058
@xiangfu0 please 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: 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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r2467169973
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -76,9 +75,8 @@ public class TableConfigBuilder {
@Deprecated
private String _segmentPushFrequency;
- // TODO: Remove 'DEFAULT_SEGMENT_PUSH_TYPE' in the future major release.
@Deprecated
- private String _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
Review Comment:
As well the _segmentPushFrequency
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-3444192970
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17072?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.26%. Comparing base ([`549a47c`](https://app.codecov.io/gh/apache/pinot/commit/549a47ccec05de2c9533dd40e24c3de472e099a5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`375ad26`](https://app.codecov.io/gh/apache/pinot/commit/375ad266b2faf6eb1b2754a65640bdd47aa3f6d1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17072?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...he/pinot/spi/utils/builder/TableConfigBuilder.java](https://app.codecov.io/gh/apache/pinot/pull/17072?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Futils%2Fbuilder%2FTableConfigBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvYnVpbGRlci9UYWJsZUNvbmZpZ0J1aWxkZXIuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (549a47c) and HEAD (375ad26). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (549a47c) | HEAD (375ad26) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17072 +/- ##
============================================
- Coverage 63.47% 56.26% -7.22%
+ Complexity 1424 701 -723
============================================
Files 3086 2419 -667
Lines 182340 135186 -47154
Branches 27979 21605 -6374
============================================
- Hits 115746 76065 -39681
+ Misses 57662 52744 -4918
+ Partials 8932 6377 -2555
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17072/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/17072/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/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <0.00%> (-7.19%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <0.00%> (-7.22%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <0.00%> (-7.21%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <0.00%> (-0.05%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17072/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/17072?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).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17072:
URL: https://github.com/apache/pinot/pull/17072#discussion_r2467169243
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -76,9 +75,8 @@ public class TableConfigBuilder {
@Deprecated
private String _segmentPushFrequency;
- // TODO: Remove 'DEFAULT_SEGMENT_PUSH_TYPE' in the future major release.
@Deprecated
- private String _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
Review Comment:
I thought we should remove this `_segmentPushType` and all the corresponding usage ?
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-4815883665
@xiangfu0 Fixed the Binary Compatibility Check failure:
**Root cause**: `japicmp --exclude @file` matches patterns against fully-qualified **class names** only. The `Class#method` syntax in `japicmp-exclude.xml` is never matched against a class name, so the exclusions were silently ignored and the removed methods were still flagged.
**Fix**: Replaced the six `Class#method` entries with two class-level entries:
```
org.apache.pinot.spi.config.table.SegmentsValidationAndRetentionConfig
org.apache.pinot.spi.utils.builder.TableConfigBuilder
```
This correctly suppresses the binary incompatibility errors for the intentionally removed deprecated methods.
--
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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-4889834849
Hi @Jackie-Jiang @xiangfu0 @yashmayya — rebased onto latest master to resolve the merge conflict in `TableConfigBuilder.java`. The branch now cherry-picks cleanly on top of master (2 commits: the removal of deprecated `segmentPushType`/`segmentPushFrequency` APIs + the `japicmp-exclude.xml` fix). CI is running. Kindly re-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: 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] Remove deprecated DEFAULT_SEGMENT_PUSH_TYPE constant [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #17072:
URL: https://github.com/apache/pinot/pull/17072#issuecomment-4889961735
Fixed a compile error: `MaterializedViewAnalyzer.java` still referenced the now-deleted `getSegmentPushType()`. Updated `resolveSegmentPushType()` to only check the modern `BatchIngestionConfig` path — the controller migration (`TableConfigUtils.convertFromLegacyTableConfig`) already moves legacy values on load. CI re-running.
--
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