liufangqi commented on pull request #17958: URL: https://github.com/apache/flink/pull/17958#issuecomment-990645530
> In general I agree with the sentiment of the patch. Few things that need to be addressed: > > * As the whole hadoop ecosystem is slowly phasing out and lot of users actually use Flink without hadoop, we shouldn't leak any hadoop specific code into the runtime. Making this change local to `flink-hadoop-fs` would be a better fit. > * It would be great to get rid of the reflections. I guess the simplest option would be compiling against newer version of hadoop and catching `NoClassDefFound` exception in case of older versions, but this requires opening a discussion on ML and getting an agreement first. > * There are no tests that validate this feature and that would be preventing future regressions. Based on the premise that hadoop version is 2.8.4, pre-write the code in the new commit (hadoop version update PR:https://github.com/apache/flink/pull/18059): 1. remove the reflctions. 2. create a new class named HadoopOptions. 3. pass the caller context with the (inheritable) thread local, rather than leak some hadoop specific code into the runtime. 4. add a unit test for the HadoopFsFactory. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
