You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2018/07/15 17:36:39 UTC

[GitHub] flink issue #6302: [FLINK-9061][checkpointing] add entropy to s3 path for be...

Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/6302
  
    Thanks for this contribution, that's a valuable fix. I have a few thoughts and suggestions on how we might improve the feature a bit still:
    
      - Can we get id of the `commons-text` dependency? The fewer dependencies, the fewer possible problems for users due to dependency clashes. It seems a bit heavy to add a new library for just one random string generation.
    
      - The feature is configured through additional constructor parameters. I am wondering if we may want to move this to the `Configuration`. That would allow the "ops side of things" to configure this for a setup (setting entropy key and checkpoints directory) without needing everyone that writes a Flink program to be aware of this.
    
      - If I read the code correctly, the code logs warnings for every file in case the feature is not activated. That will probably confuse a lot of users and make them dig into whether they have a wrong setup, when they simply don't use this new feature.



---