davidradl commented on code in PR #25901:
URL: https://github.com/apache/flink/pull/25901#discussion_r1904363832


##########
flink-runtime/src/main/java/org/apache/flink/runtime/asyncprocessing/functions/DeclaringKeyedProcessFunction.java:
##########
@@ -38,17 +37,33 @@
  * and register further timers.
  *
  * <p><b>NOTE:</b> Access to keyed state and timers (which are also scoped to 
a key) is only
- * available if the {@code AsyncKeyedProcessFunction} is applied on a {@code 
KeyedStream}.
+ * available if the {@code DeclaringKeyedProcessFunction} is applied on a 
{@code KeyedStream}.
  *
  * @param <K> Type of the key.
  * @param <I> Type of the input elements.
  * @param <O> Type of the output elements.
  */
 @Internal
-public abstract class AsyncKeyedProcessFunction<K, I, O> extends 
AbstractAsyncStatefulRichFunction {
+public abstract class DeclaringKeyedProcessFunction<K, I, O> extends 
KeyedProcessFunction<K, I, O> {

Review Comment:
   It seems bad practice to rename a class. I suggest deprecating the old class 
and introducing an new class.
   
   The class is an asynchronous version of `KeyedProcessFunction` so the old 
class name seems to represent what it does. If we need a new name so as not to 
clash with the old one, then maybe `AsyncDeclaringKeyedProcessFunction`.
   
   Declaring is used in the class name. If this is important then I think we 
should use this word in the class description to help readers of the code 
understand the class name.
   
    



-- 
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]

Reply via email to