Copilot commented on code in PR #16824:
URL: https://github.com/apache/pinot/pull/16824#discussion_r2351629302
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java:
##########
@@ -384,13 +384,13 @@ private List<String>
getSegmentsToDeleteFromDeepstore(String tableNameWithType,
*
* @param tableNameWithType Name of the offline table
* @param retentionStrategy Strategy to determine if a segment should be
purged
- * @param segmentsToExclude List of segment names that should be excluded
from deletion
+ * @param segmentsToExclude Set of segment names that should be excluded
from deletion
* @return List of segment names that should be deleted from deepstore
* @throws IOException If there's an error accessing the filesystem
*/
- private List<String> findUntrackedSegmentsToDeleteFromDeepstore(String
tableNameWithType,
- RetentionStrategy retentionStrategy, List<String> segmentsToExclude,
- RetentionStrategy untrackedSegmentsRetentionStrategy)
+ @VisibleForTesting
+ List<String> findUntrackedSegmentsToDeleteFromDeepstore(String
tableNameWithType, RetentionStrategy retentionStrategy,
Review Comment:
The method visibility has been changed from private to package-private with
@VisibleForTesting annotation, but this creates an inconsistent API design
pattern. Consider keeping the method private and using reflection in tests or
creating a separate testable helper method to avoid exposing internal
implementation details.
```suggestion
private List<String> findUntrackedSegmentsToDeleteFromDeepstore(String
tableNameWithType, RetentionStrategy retentionStrategy,
```
--
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]
