flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From StephanEwen <...@git.apache.org>
Subject [GitHub] flink issue #4241: [FLINK-7015] [streaming] separate OperatorConfig from Str...
Date Mon, 03 Jul 2017 17:53:51 GMT
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/4241
  
    In general, it is a good idea to clean that up a bit. That code needed some refactoring.
    
    As @aljoscha pointed out, that is an intricate part, so we need to review that carefully.
To be able to review that, we need a good description of what the refactoring did. Trying
to understand that from the code alone will take a lot of time and most likely slow down the
merging of this by a lot.
    
    I would suggest to write
      - what exactly was addressed
      - what is the new design
      - does it change behavior, or is it only code abstraction internally
    
    
    As another point: I just took a quick look at the `OperatorConfig`. I think I would be
nice to move away from the model of having a Map of "configKey" to values, but just to make
this a serializable class with the respective fields. That would be simpler to understand
and maintain.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message