pnowojski commented on issue #8759: [FLINK-12868][yarn] Fix yarn cluster can 
not be deployed if plugins dir does not exist
URL: https://github.com/apache/flink/pull/8759#issuecomment-503931602
 
 
   Thanks for the reviews/comments @azagrebin and @TisonKun.
   
   I would prefer to avoid treating anything related to this plugin directory 
as "obligatory". Some arguments:
   - `lib` can not be empty for Flink to work properly, while `plugins` can
   - if we make the contract of directory must exist when env variable is set, 
that would be cleaner, however it is much more risky. We have pretty weak test 
coverage when it comes to the deployments methods, so we could easily miss some 
place that mishandles the directory. If that happens, we brake existing setups, 
even when users didn't want to use `plugins` at all. 
   - there are only 4 places in the code which are touching the env 
variable/plugins directory and we can make sure that they correctly handle non 
existing directory. While on the other hand, there are many many more code 
paths that are leading us to those 4 places in which we would have to be 100% 
sure that env variable and plugins directory is handled correctly, **even if 
the plugins directory is empty** - even if it's empty, it would still have to 
be shipped/copied/created
   - "the failing fast" would improve user experience only if a user by passed 
our shell scripts and used a custom value for the plugin env variable - that's 
super rare
   - "failing later" (as it is implemented now) means that user gets a warning 
that directory doesn't exist and he later would get an exception "FileSystem X 
was not found" or "MetricReporter Y was not found" 
   
   So to sum up. "failing fast" is nicer, but it is outweighed in my opinion by 
too high risk that we brake some existing deployment (because of poor existing 
test coverage). With "failing later", true the failure happens later but the 
error is still easily understandable plus we are logging a warning.
   
   After discussing this with @zentol and @azagrebin, I agree that one more 
missing thing here, is that we should create an empty `plugins` directory in 
`flink-dist`. In that way as long as users are using provided by us shell 
scripts (vast majority), they would be not affected by this "fail fast" vs 
"fail late" issue. Also it would be much more explicit and better user 
experience if he doesn't have to figure out how to name the root plugins 
directory, because it will already exist.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to