[GitHub] [flink] rkhachatryan commented on pull request #18963: [FLINK-26450][fs] Adds error handling to FileStateHandle.discardState

[GitBox <gi...@apache.org>]

rkhachatryan commented on pull request #18963:
URL: https://github.com/apache/flink/pull/18963#issuecomment-1057871273


   I have similar concerns regarding the tests as @dawidwys. production code LGTM.
   
   `TestingFileSystem` is needed I guess to register the file system; and this is needed to test "return false" case (where non-existent file can't be used  because it can throw an exception instead of returning false).
   
   1. `TestingFileSystem` might be brittle because it has a lot of unsupported operations. Can't we delegate everything except `delete` to some "real" FS?
   2. Callbacks make it less readdable, and mutable callbacks - error-prone, why do we need to reset them?
   3. I'm wondering whether a local FS can be picked up instead of a testing FS for some reason - should we guard against it using some "test://" scheme?


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org