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
| @@ -67,6 +72,31 @@ | |||
| private final ArrayList<AnalyticJob> _secondRetryQueue = new ArrayList<AnalyticJob>(); | |||
|
|
|||
| 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.
| @@ -145,8 +167,8 @@ private URL getTaskAttemptURL(String dagId, String taskId, String attemptId) thr | |||
|
|
|||
| private String _timelineWebAddr; | |||
|
|
|||
| 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 URLFactory _urlFactory; | ||
| private JSONFactory _jsonFactory; | ||
| private String _timelineWebAddr; | ||
|
|
||
| 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?
| private static final String FETCH_INITIAL_WINDOW_MS = "drelephant.analysis.fetch.initial.windowMillis"; | ||
|
|
||
|
|
||
| //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 URLFactory _urlFactory; | ||
| private JSONFactory _jsonFactory; | ||
| 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 |
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)