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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
