Enabling Dr Elephant to work with YARN in HTTPS mode #475
Conversation
Enabling Dr Elephant to work with YARN in HTTPS mode
app/com/linkedin/drelephant/analysis/AnalyticJobGeneratorHadoop2.java
Outdated
Show resolved
Hide resolved
|
|
||
| public void updateResourceManagerAddresses() { | ||
|
|
||
| String rm_http_policy = configuration.get(RM_HTTP_POLICY); |
There was a problem hiding this comment.
please follow naming convention
There was a problem hiding this comment.
Please move your code to seperate method , as it makes this method really long
|
@shahrukhkhan489 Add unit test cases to check the functionality |
|
|
||
| //Assigning URL's a header of http:// or https:// depending if YARN https is enabled or not | ||
| private static String RM_URL_HEADER; | ||
| private static String RESOURCE_MANAGER_ADDRESS; |
There was a problem hiding this comment.
Any specific reason to make these 3 variables static? In fact, RESOURCE_MANAGER_ADDRESS may not even need to be a member variable.
|
|
||
| private URLFactory(String hserverAddr) throws IOException { | ||
| _timelineWebAddr = "http://" + hserverAddr + "/ws/v1/timeline"; | ||
| private URLFactory(String hserverAddr,String RM_URL_HEADER) throws IOException { |
There was a problem hiding this comment.
Kindy follow naming convention for method parameters.
|
|
||
| private FetcherConfigurationData _fetcherConfigurationData; | ||
| private static String RM_URL_HEADER; | ||
| private static String TIMELINE_SERVER_URL; |
There was a problem hiding this comment.
Should these be static?
|
|
||
|
|
||
| //Assigning URL's a header of http:// or https:// depending if YARN https is enabled or not | ||
| private static String RM_URL_HEADER; |
There was a problem hiding this comment.
http:// or https:// refers to URL scheme instead of header. Rename the variable name accordingly.
There was a problem hiding this comment.
Updating object name from RM_URL_HEADER to RM_URL_SCHEME
| private String _timelineWebAddr; | ||
|
|
||
| private FetcherConfigurationData _fetcherConfigurationData; | ||
| private static String RM_URL_HEADER; |
There was a problem hiding this comment.
Should be TIMELINE_SERVER_URL_PREFIX/SCHEME
|
@shahrukhkhan489 can you check the Travis CI build failure. It has failed due to drop in coverage |
3 participants
This pull request is to address #472
Configuration value of the property -
yarn.http.policywhich can have either of the values -HTTP_ONLY / HTTPS_ONLY, defines whether YARN is in HTTPS modeChanges Addressed to below files -
If statement inserted to check the configuration value of the property -
yarn.http.policywhich can have either of the values -HTTP_ONLY/HTTPS_ONLY. Based on the value below properties need to be addressedRESOURCE_MANAGER_ADDRESS as
yarn.resourcemanager.webapp.addressoryarn.resourcemanager.webapp.https.addressRM_NODE_STATE_URL as
http://%s/ws/v1/cluster/info or https://%s/ws/v1/cluster/infosucceededAppsURL and failedAppsURL as
new URL("http://" + _resourceManagerAddress)ornew URL("https://" + _resourceManagerAddress)