incubator-myriad
incubator-myriad copied to clipboard
apache
MYRIAD-184 RM Ports are Hardcoded in NMExecutorCLGenImpl.java
Is this a good idea? Seems like we're adding more config to Myriad maybe we should just throw a runtime exception?
As in assert that the port be the default port as we are going now, where we throw an exception should there be no response at the given default port address?
8088 is yarns default HTTP port and 8090 is the HTTPS port. If neither are in the yarn-site.xml they'll revert to those. The original check in code was for sanity in case the yarn configuration didn't have those values and returned a null.
Adding additional config here could lead to a conflict between yarn and Myriad's perception of the correct port.
The assert would be do we get a value (not null or empty).
A good question would be how are people currently deploying their myriad servers, if they are using some kind of configuration management tool, this makes live much easier if you want to change the port, as you can just create a template file where you can then override the yarn http port.
If a person is using everything the way using the default configs from both yarn and myriad, then the config is already set as I had done.
That said if you think its best to go the other way then how would you recommend making such an assertion, is there a yarn endpoint we can call to see if that port is the default, is it even worth making such a request if it has to be done over http.
This makes sense to render the port configurable and implement the getter with Optional.of(
I agree with @DarinJ. I think it is not necessary to enter the configuration because we cover the use cases of no configuration with the default case and use cases of configuration with: yarn.resourcemanager.webapp.https.address
