Akanksha-kedia commented on code in PR #18472:
URL: https://github.com/apache/pinot/pull/18472#discussion_r3381804710
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:
##########
@@ -220,20 +212,16 @@ public TableConfigBuilder
setLineageEntryCleanupRetentionPeriod(String lineageEn
/**
* @deprecated Use {@code segmentIngestionType} from {@link
IngestionConfig#getBatchIngestionConfig()}
*/
+ @Deprecated
public TableConfigBuilder setSegmentPushType(String segmentPushType) {
- if (REFRESH_SEGMENT_PUSH_TYPE.equalsIgnoreCase(segmentPushType)) {
- _segmentPushType = REFRESH_SEGMENT_PUSH_TYPE;
- } else {
- _segmentPushType = DEFAULT_SEGMENT_PUSH_TYPE;
- }
return this;
Review Comment:
Thank you both for the thorough feedback. You're right — turning the
deprecated setters into silent no-ops is a backward-compatibility break.
@Jackie-Jiang your guidance that the fields are now in
`BatchIngestionConfig` is the right direction. I'll rework this PR to:
1. Keep the deprecated setters functional (write-through to
`BatchIngestionConfig` so existing callers are not silently broken)
2. Add a deprecation log warning to encourage migration
Would appreciate a quick pointer to the exact `BatchIngestionConfig` fields
these should map to so I can get the fix right. I'll update the PR once that's
confirmed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
