XComp commented on code in PR #24387: URL: https://github.com/apache/flink/pull/24387#discussion_r1503776636
########## flink-core/src/main/java/org/apache/flink/api/common/state/StateTtlConfig.java: ########## @@ -531,7 +531,7 @@ public long getQueryTimeAfterNumEntries() { return queryTimeAfterNumEntries; } - public Time getPeriodicCompactionTime() { + public Duration getPeriodicCompactionTime() { Review Comment: ```suggestion public Duration getPeriodicCompactionDuration() { return periodicCompactionTime; } /** * Returns the configured periodic compaction time. * * @deprecated Use {@link #getPeriodicCompactionDuration()}, instead. */ @Deprecated public Time getPeriodicCompactionTime() { return Time.fromDuration(periodicCompactionTime); } ``` The proper way of deprecating API based on [FLIP-321](https://cwiki.apache.org/confluence/display/FLINK/FLIP-321%3A+Introduce+an+API+deprecation+process) is providing the new version but keeping the old version (labeled as deprecated). ########## flink-core/src/main/java/org/apache/flink/api/common/state/StateTtlConfig.java: ########## @@ -341,7 +341,7 @@ public Builder cleanupInRocksdbCompactFilter(long queryTimeAfterNumEntries) { */ @Nonnull public Builder cleanupInRocksdbCompactFilter( - long queryTimeAfterNumEntries, Time periodicCompactionTime) { + long queryTimeAfterNumEntries, Duration periodicCompactionTime) { Review Comment: same here, we should still provide the old version of the API. ########## flink-python/pyflink/datastream/state.py: ########## Review Comment: I guess, analogously, to the Java version, deprecated API should survive for at least one minor release. ########## flink-core/src/main/java/org/apache/flink/api/common/state/StateTtlConfig.java: ########## @@ -497,7 +497,7 @@ public static class RocksdbCompactFilterCleanupStrategy * Default value is 30 days so that every file goes through the compaction process at least * once every 30 days if not compacted sooner. */ - static final Time DEFAULT_PERIODIC_COMPACTION_TIME = Time.days(30); + static final Duration DEFAULT_PERIODIC_COMPACTION_TIME = Duration.ofDays(30); Review Comment: ```suggestion static final Duration DEFAULT_PERIODIC_COMPACTION_DURATION = Duration.ofDays(30); /** @deprecated Use {@link #DEFAULT_PERIODIC_COMPACTION_DURATION}, instead. */ @Deprecated static final Time DEFAULT_PERIODIC_COMPACTION_TIME = Time.fromDuration(DEFAULT_PERIODIC_COMPACTION_DURATION); ``` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org