Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-alpha2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We have several configurations are hard-coded, for example, RM/ATS addresses, we should make them configurable.

      1. YARN-4514-YARN-3368.8.patch
        49 kB
        Sunil G
      2. YARN-4514-YARN-3368.7.patch
        49 kB
        Sunil G
      3. YARN-4514-YARN-3368.6.patch
        45 kB
        Sunil G
      4. YARN-4514-YARN-3368.5.patch
        45 kB
        Sunil G
      5. YARN-4514-YARN-3368.4.patch
        12 kB
        Sunil G
      6. YARN-4514-YARN-3368.3.patch
        11 kB
        Sunil G
      7. YARN-4514-YARN-3368.2.patch
        11 kB
        Sunil G
      8. YARN-4514-YARN-3368.1.patch
        12 kB
        Sunil G

        Activity

        Hide
        sunilg Sunil G added a comment -

        Attaching an initial version patch to cleanup few hardcodings.
        Wangda Tan Could you please take a look.

        Show
        sunilg Sunil G added a comment - Attaching an initial version patch to cleanup few hardcodings. Wangda Tan Could you please take a look.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Sunil G,

        Thanks for working on this, looks good to me. Will try this after YARN-4849 committed.

        Show
        leftnoteasy Wangda Tan added a comment - Sunil G , Thanks for working on this, looks good to me. Will try this after YARN-4849 committed.
        Hide
        sunilg Sunil G added a comment -

        Attaching an updated patch after changing RM Web default port to 8088.

        Show
        sunilg Sunil G added a comment - Attaching an updated patch after changing RM Web default port to 8088.
        Hide
        varun_saxena Varun Saxena added a comment -

        IIUC, we expect this configuration to be configured by user during deployment. Right ?

        If that's the case shouldn't the namespaces like ws/v1/cluster, etc. move to constants.js, the file I had created in YARN-4517.
        I am not too sure if it be required for users to configure namespaces. These sound more like constants.
        Maybe the only config which they may require is whether they are connecting to ATSv1 or ATSv2, which I think can be added in some ATSv2 JIRA.
        Sunil G, what do you think ?

        Also would we want to put config.js in a separate folder named something like config ? I had placed it outside(under app folder) in YARN-4517 because it was mainly added for convenience and we had this JIRA open too. Basically we need to decide where we want to keep it while packaging.

        Correct me if I am wrong. I think we wont require localBaseUrl(which is corsproxy URL) in normal production cluster.
        So maybe we can have a separate config for scheme(http/https). And do not consider corsproxy URL for production. I think maybe we can use ENV.baseUrl in config/environment.js for this. And set ENV.baseUrl to "" for production.

        Show
        varun_saxena Varun Saxena added a comment - IIUC, we expect this configuration to be configured by user during deployment. Right ? If that's the case shouldn't the namespaces like ws/v1/cluster, etc. move to constants.js, the file I had created in YARN-4517 . I am not too sure if it be required for users to configure namespaces. These sound more like constants. Maybe the only config which they may require is whether they are connecting to ATSv1 or ATSv2, which I think can be added in some ATSv2 JIRA. Sunil G , what do you think ? Also would we want to put config.js in a separate folder named something like config ? I had placed it outside(under app folder) in YARN-4517 because it was mainly added for convenience and we had this JIRA open too. Basically we need to decide where we want to keep it while packaging. Correct me if I am wrong. I think we wont require localBaseUrl(which is corsproxy URL) in normal production cluster. So maybe we can have a separate config for scheme(http/https). And do not consider corsproxy URL for production. I think maybe we can use ENV.baseUrl in config/environment.js for this. And set ENV.baseUrl to "" for production.
        Hide
        sunilg Sunil G added a comment -

        we expect this configuration to be configured by user during deployment. Right ?

        I say admin. But more or less its a user.

        namespaces like ws/v1/cluster, etc. move to constants.js

        I still say these are config than constants. .Because some of the rest namespace end points can be added in future, and may get changed too. So it ll help in upgrades etc. I looked TEZ code, and they use this as config too. Thoughts?

        maybe we can have a separate config for scheme(http/https)

        Yes, I will address the same.

        I think we wont require localBaseUrl(which is corsproxy URL) in normal production cluster

        We can use localBaseUrl and keep it as empty for now. And for tests, we can configure corsproxy URL in testbed.

        I will also change the config name from default to address as discussed offline.

        Show
        sunilg Sunil G added a comment - we expect this configuration to be configured by user during deployment. Right ? I say admin. But more or less its a user. namespaces like ws/v1/cluster, etc. move to constants.js I still say these are config than constants. .Because some of the rest namespace end points can be added in future, and may get changed too. So it ll help in upgrades etc. I looked TEZ code, and they use this as config too. Thoughts? maybe we can have a separate config for scheme(http/https) Yes, I will address the same. I think we wont require localBaseUrl(which is corsproxy URL) in normal production cluster We can use localBaseUrl and keep it as empty for now. And for tests, we can configure corsproxy URL in testbed. I will also change the config name from default to address as discussed offline.
        Hide
        sunilg Sunil G added a comment -

        Attaching a new patch addressing the comments from Varun Saxena.

        Varun Saxena/Wangda Tan. Could you pls take a look.

        Show
        sunilg Sunil G added a comment - Attaching a new patch addressing the comments from Varun Saxena . Varun Saxena / Wangda Tan . Could you pls take a look.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Sunil G,

        I tried this patch locally on top of YARN-4849.

        Only one minor comment. It seems to me that we should modify localBaseUrl to be "localhost:1337/" before we solve CORS problem. Otherwise it will fail in default settings because corsproxy is not used.

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Sunil G , I tried this patch locally on top of YARN-4849 . Only one minor comment. It seems to me that we should modify localBaseUrl to be "localhost:1337/" before we solve CORS problem. Otherwise it will fail in default settings because corsproxy is not used.
        Hide
        sunilg Sunil G added a comment -

        Thank you Wangda Tan for the comments. Yes, I agree. Its a valid point. We will have it also configured by default.

        Show
        sunilg Sunil G added a comment - Thank you Wangda Tan for the comments. Yes, I agree. Its a valid point. We will have it also configured by default.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Have to resolve and reopen to set status to be patch available.

        Show
        leftnoteasy Wangda Tan added a comment - Have to resolve and reopen to set status to be patch available.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        -1 patch 0m 5s YARN-4514 does not apply to YARN-3368. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



        Subsystem Report/Notes
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797310/YARN-4514-YARN-3368.4.patch
        JIRA Issue YARN-4514
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/10987/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s YARN-4514 does not apply to YARN-3368 . Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797310/YARN-4514-YARN-3368.4.patch JIRA Issue YARN-4514 Console output https://builds.apache.org/job/PreCommit-YARN-Build/10987/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 1m 51s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 asflicense 0m 19s Patch does not generate ASF License warnings.
        2m 26s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:e7ef482
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797310/YARN-4514-YARN-3368.4.patch
        JIRA Issue YARN-4514
        Optional Tests asflicense
        uname Linux f6f4ac274971 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-3368 / e7ef482
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/10998/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 1m 51s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 whitespace 0m 0s Patch has no whitespace issues. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 2m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:e7ef482 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797310/YARN-4514-YARN-3368.4.patch JIRA Issue YARN-4514 Optional Tests asflicense uname Linux f6f4ac274971 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-3368 / e7ef482 modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui Console output https://builds.apache.org/job/PreCommit-YARN-Build/10998/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        +1 to latest patch.

        Varun Saxena, could you take a look at latest patch?

        Show
        leftnoteasy Wangda Tan added a comment - +1 to latest patch. Varun Saxena , could you take a look at latest patch?
        Hide
        varun_saxena Varun Saxena added a comment -

        Wangda Tan, the patch as such looks fine to me.

        I have a couple of comments though.
        For the config "localBaseUrl" , we have to configure it with a slash in the end. Otherwise URL would not be formed properly. A user can easily miss configuring it that way if he changes corsproxy address for instance. It is more likely that somebody would configure it as localhost:1337 instead of localhost:1337/
        Although it can be mentioned in config file comments but it just looks a bit weird.
        I think in each of the adapter files we can put a / by ourselves.
        Moreover the config names have been named as URLs'. timelineWebUrl, rmWebUrl, etc. URL contains a protocol identifier too. Maybe name it as timelineWebAddress, rmWebAddress and so on.
        What do you think ?

        Also I had one earlier comment. Do you think namespaces should be put in configuration file. Shouldnt they be constants ? Sunil was of the opinion that this would make it easier when we are upgrading. You can take a call on it. I felt these can be constants.

        These are minor comments though and can be taken care later while doing some other JIRA. If its blocking merge to trunk, I think you can go ahead and commit it.

        Show
        varun_saxena Varun Saxena added a comment - Wangda Tan , the patch as such looks fine to me. I have a couple of comments though. For the config "localBaseUrl" , we have to configure it with a slash in the end. Otherwise URL would not be formed properly. A user can easily miss configuring it that way if he changes corsproxy address for instance. It is more likely that somebody would configure it as localhost:1337 instead of localhost:1337/ Although it can be mentioned in config file comments but it just looks a bit weird. I think in each of the adapter files we can put a / by ourselves. Moreover the config names have been named as URLs'. timelineWebUrl, rmWebUrl, etc. URL contains a protocol identifier too. Maybe name it as timelineWebAddress, rmWebAddress and so on. What do you think ? Also I had one earlier comment. Do you think namespaces should be put in configuration file. Shouldnt they be constants ? Sunil was of the opinion that this would make it easier when we are upgrading. You can take a call on it. I felt these can be constants. These are minor comments though and can be taken care later while doing some other JIRA. If its blocking merge to trunk, I think you can go ahead and commit it.
        Hide
        sunilg Sunil G added a comment -

        Although it can be mentioned in config file comments but it just looks a bit weird. I think in each of the adapter files we can put a / by ourselves.

        There is one issue with this. Going forward if we are moving out from CORS, then we can give it empty. So URL will be like below:
        Config.envDefaults.protocolScheme + Config.envDefaults.localBaseUrl + Config.envDefaults.rmWebURL which will be translated as
        Config.envDefaults.protocolScheme = "http://"
        Config.envDefaults.localBaseUrl = ""
        Config.envDefaults.rmWebURL = "<ip>:<port>"

        So URL will come correctly. If we set / by ourselves, then it will be an extra config only for CORS proxy and we ll have issues. SO i think we can mentioned on comment itself.

        Maybe name it as timelineWebAddress, rmWebAddress and so on.

        Make sense to me. I will update.

        For other comments, I will wait for Wangda Tan take also as mentioned by Varun Saxena

        Show
        sunilg Sunil G added a comment - Although it can be mentioned in config file comments but it just looks a bit weird. I think in each of the adapter files we can put a / by ourselves. There is one issue with this. Going forward if we are moving out from CORS, then we can give it empty. So URL will be like below: Config.envDefaults.protocolScheme + Config.envDefaults.localBaseUrl + Config.envDefaults.rmWebURL which will be translated as Config.envDefaults.protocolScheme = "http://" Config.envDefaults.localBaseUrl = "" Config.envDefaults.rmWebURL = "<ip>:<port>" So URL will come correctly. If we set / by ourselves, then it will be an extra config only for CORS proxy and we ll have issues. SO i think we can mentioned on comment itself. Maybe name it as timelineWebAddress, rmWebAddress and so on. Make sense to me. I will update. For other comments, I will wait for Wangda Tan take also as mentioned by Varun Saxena
        Hide
        varun_saxena Varun Saxena added a comment -

        Cant we have "host" in each of the adapter files as a computed property ?
        Check if localBaseUrl or localBaseAddress is empty. If not, insert an additional /.
        I havent tried it but this should work. Thoughts ?

        My main concern here is / can be easily missed while configuring. It wont be intuitive to anyone who is configuring. However, it can be mentioned in comments too. That / in the end is necessary if config is not empty.

        Show
        varun_saxena Varun Saxena added a comment - Cant we have "host" in each of the adapter files as a computed property ? Check if localBaseUrl or localBaseAddress is empty. If not, insert an additional /. I havent tried it but this should work. Thoughts ? My main concern here is / can be easily missed while configuring. It wont be intuitive to anyone who is configuring. However, it can be mentioned in comments too. That / in the end is necessary if config is not empty.
        Hide
        sunilg Sunil G added a comment -

        It looks like this patch is committed to YARN-3368 branch. I will have another ticket to handle those minor comments from Varun.

        Show
        sunilg Sunil G added a comment - It looks like this patch is committed to YARN-3368 branch. I will have another ticket to handle those minor comments from Varun.
        Hide
        varun_saxena Varun Saxena added a comment -

        By computed property I meant something like this.

        host: function() {
          var host = Config.envDefaults.protocolScheme;
          if (Config.envDefaults.localBaseUrl.length > 0) {
            host = host + Config.envDefaults.localBaseUrl + '/';
          }
          host = host + Config.envDefaults.rmWebURL;
          return host;
        }.property(),
        
        Show
        varun_saxena Varun Saxena added a comment - By computed property I meant something like this. host: function() { var host = Config.envDefaults.protocolScheme; if (Config.envDefaults.localBaseUrl.length > 0) { host = host + Config.envDefaults.localBaseUrl + '/'; } host = host + Config.envDefaults.rmWebURL; return host; }.property(),
        Hide
        varun_saxena Varun Saxena added a comment -

        Oh. Yeah these are minor comments. We can handle these in a separate JIRA. Or just fix it in one of the JIRAs' which you are currently handling.

        Show
        varun_saxena Varun Saxena added a comment - Oh. Yeah these are minor comments. We can handle these in a separate JIRA. Or just fix it in one of the JIRAs' which you are currently handling.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Sorry I pushed the change unconsciously while doing rebase, removed YARN-4514 from branch-3368 just now.

        When I look at tez-ui2, their configuration file is a configs.env instead of JS. which could be kept after build: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/config/configs.env.

        And it adds reference to the config file from index.html: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/app/index.html

        If it is possible, we should follow the pattern, otherwise the configs.js will be appended to other JS files so we cannot modify it after packaging.

        Computed value is a good idea: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/app/services/hosts.js

        And we may not need localBaseUrl, since it is a workaround before we fix CORS issues in YARN.

        Show
        leftnoteasy Wangda Tan added a comment - Sorry I pushed the change unconsciously while doing rebase, removed YARN-4514 from branch-3368 just now. When I look at tez-ui2, their configuration file is a configs.env instead of JS. which could be kept after build: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/config/configs.env . And it adds reference to the config file from index.html: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/app/index.html If it is possible, we should follow the pattern, otherwise the configs.js will be appended to other JS files so we cannot modify it after packaging. Computed value is a good idea: https://github.com/apache/tez/blob/master/tez-ui2/src/main/webapp/app/services/hosts.js And we may not need localBaseUrl, since it is a workaround before we fix CORS issues in YARN.
        Hide
        sunilg Sunil G added a comment -

        tez-ui2, their configuration file is a configs.env instead of JS. which could be kept after build

        They still have default-config.js. But main configs are in env. I will make these changes.

        Computed value is a good idea

        This is a good option. Thanks for pointing out. Let me make changes in this line.

        And we may not need localBaseUrl, since it is a workaround before we fix CORS issues in YARN.

        But I think still we need it for now as we are testing. I think in formal version, we can keep this config as empty. For now, we can keep a default value. Thoughts?

        Show
        sunilg Sunil G added a comment - tez-ui2, their configuration file is a configs.env instead of JS. which could be kept after build They still have default-config.js. But main configs are in env. I will make these changes. Computed value is a good idea This is a good option. Thanks for pointing out. Let me make changes in this line. And we may not need localBaseUrl, since it is a workaround before we fix CORS issues in YARN. But I think still we need it for now as we are testing. I think in formal version, we can keep this config as empty. For now, we can keep a default value. Thoughts?
        Hide
        leftnoteasy Wangda Tan added a comment -

        But I think still we need it for now as we are testing. I think in formal version, we can keep this config as empty. For now, we can keep a default value. Thoughts?

        I don't have strong opinion on this. It's your call

        Show
        leftnoteasy Wangda Tan added a comment - But I think still we need it for now as we are testing. I think in formal version, we can keep this config as empty. For now, we can keep a default value. Thoughts? I don't have strong opinion on this. It's your call
        Hide
        sunilg Sunil G added a comment -

        I think its better to keep this config for now. And I can formulate a URL by this new way suggested. So it will cover Varun Saxena scenario too.
        I will work on these changes now and will share patch after a few tests. Thank You.

        Show
        sunilg Sunil G added a comment - I think its better to keep this config for now. And I can formulate a URL by this new way suggested. So it will cover Varun Saxena scenario too. I will work on these changes now and will share patch after a few tests. Thank You.
        Hide
        sunilg Sunil G added a comment -

        Wangda Tan and Varun Saxena

        I have made some more changes in code layout to accommodate config support

        • I had to bump up ember version to 2.2.0 for smooth factoring (needed to use initializer and services)
        • Also refactored adapter part by introducing abstract class. So we could move few common code from all adapter.
        • package and bower config is now similar to tez ui2.

        Pls share your thoughts and feedback. UI from outer perspective is still same, no changes.

        Show
        sunilg Sunil G added a comment - Wangda Tan and Varun Saxena I have made some more changes in code layout to accommodate config support I had to bump up ember version to 2.2.0 for smooth factoring (needed to use initializer and services) Also refactored adapter part by introducing abstract class. So we could move few common code from all adapter. package and bower config is now similar to tez ui2. Pls share your thoughts and feedback. UI from outer perspective is still same, no changes.
        Hide
        sunilg Sunil G added a comment -

        Few more changes done:

        • Updated LICENSE file with changed versions.
        • Cleaned up code by removing some debug logs added

        Wangda Tan and Varun Saxena, pls help to check the patch.

        Show
        sunilg Sunil G added a comment - Few more changes done: Updated LICENSE file with changed versions. Cleaned up code by removing some debug logs added Wangda Tan and Varun Saxena , pls help to check the patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 2m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        0 mvndep 3m 51s Maven dependency ordering for branch
        0 mvndep 0m 15s Maven dependency ordering for patch
        -1 whitespace 0m 0s The patch has 2 line(s) with tabs.
        -1 asflicense 0m 34s Patch generated 5 ASF License warnings.
        7m 16s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:e35bf0f
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798281/YARN-4514-YARN-3368.6.patch
        JIRA Issue YARN-4514
        Optional Tests asflicense
        uname Linux ec7542f9784e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-3368 / e35bf0f
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/11052/artifact/patchprocess/whitespace-tabs.txt
        asflicense https://builds.apache.org/job/PreCommit-YARN-Build/11052/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: .
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/11052/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 2m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. 0 mvndep 3m 51s Maven dependency ordering for branch 0 mvndep 0m 15s Maven dependency ordering for patch -1 whitespace 0m 0s The patch has 2 line(s) with tabs. -1 asflicense 0m 34s Patch generated 5 ASF License warnings. 7m 16s Subsystem Report/Notes Docker Image:yetus/hadoop:e35bf0f JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798281/YARN-4514-YARN-3368.6.patch JIRA Issue YARN-4514 Optional Tests asflicense uname Linux ec7542f9784e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-3368 / e35bf0f whitespace https://builds.apache.org/job/PreCommit-YARN-Build/11052/artifact/patchprocess/whitespace-tabs.txt asflicense https://builds.apache.org/job/PreCommit-YARN-Build/11052/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11052/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sunilg Sunil G added a comment -

        Fixed asf warnings...

        Show
        sunilg Sunil G added a comment - Fixed asf warnings...
        Hide
        sunilg Sunil G added a comment -

        It seems jenkins is not triggered. Wangda Tan, could u pls help to kick jenkins manually.

        Show
        sunilg Sunil G added a comment - It seems jenkins is not triggered. Wangda Tan , could u pls help to kick jenkins manually.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        0 mvndep 2m 13s Maven dependency ordering for branch
        0 mvndep 0m 15s Maven dependency ordering for patch
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 asflicense 0m 19s Patch does not generate ASF License warnings.
        3m 20s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:e35bf0f
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798311/YARN-4514-YARN-3368.7.patch
        JIRA Issue YARN-4514
        Optional Tests asflicense
        uname Linux 6ad19ad54a1d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-3368 / e35bf0f
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: .
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/11063/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. 0 mvndep 2m 13s Maven dependency ordering for branch 0 mvndep 0m 15s Maven dependency ordering for patch +1 whitespace 0m 0s Patch has no whitespace issues. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 3m 20s Subsystem Report/Notes Docker Image:yetus/hadoop:e35bf0f JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798311/YARN-4514-YARN-3368.7.patch JIRA Issue YARN-4514 Optional Tests asflicense uname Linux 6ad19ad54a1d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-3368 / e35bf0f modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11063/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Sunil G,

        I tried to run the patch locally, it works fine for me.

        Only one minor comment:

            /*
             * Local URL. This can be empty by default. For testbed if corsproxy is used,
             * corsproxy URL can be configured here. For eg:"localhost:1337"
             */
        

        It could be more clear, such as:
        "This is empty by default. ... Turn it on while RM/ATS running on the same host... etc..."

        Show
        leftnoteasy Wangda Tan added a comment - Sunil G , I tried to run the patch locally, it works fine for me. Only one minor comment: /* * Local URL. This can be empty by default . For testbed if corsproxy is used, * corsproxy URL can be configured here. For eg: "localhost:1337" */ It could be more clear, such as: "This is empty by default. ... Turn it on while RM/ATS running on the same host... etc..."
        Hide
        sunilg Sunil G added a comment -

        Thank you Wangda Tan. Please help to check the update message in default-config.js. Attaching an updated patch

        Show
        sunilg Sunil G added a comment - Thank you Wangda Tan . Please help to check the update message in default-config.js. Attaching an updated patch
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 1m 15s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        0 mvndep 0m 50s Maven dependency ordering for branch
        0 mvndep 0m 14s Maven dependency ordering for patch
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 asflicense 0m 19s Patch does not generate ASF License warnings.
        2m 54s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:e35bf0f
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798975/YARN-4514-YARN-3368.8.patch
        JIRA Issue YARN-4514
        Optional Tests asflicense
        uname Linux ab19464933d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-3368 / e35bf0f
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: .
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/11098/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 1m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. 0 mvndep 0m 50s Maven dependency ordering for branch 0 mvndep 0m 14s Maven dependency ordering for patch +1 whitespace 0m 0s Patch has no whitespace issues. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 2m 54s Subsystem Report/Notes Docker Image:yetus/hadoop:e35bf0f JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798975/YARN-4514-YARN-3368.8.patch JIRA Issue YARN-4514 Optional Tests asflicense uname Linux ab19464933d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-3368 / e35bf0f modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11098/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 10s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        0 mvndep 2m 49s Maven dependency ordering for branch
        0 mvndep 0m 14s Maven dependency ordering for patch
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 asflicense 0m 44s Patch does not generate ASF License warnings.
        4m 15s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:4bf84af
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798975/YARN-4514-YARN-3368.8.patch
        JIRA Issue YARN-4514
        Optional Tests asflicense
        uname Linux 3ccea15df82d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-3368 / 4bf84af
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: .
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/11108/console
        Powered by Apache Yetus 0.2.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. 0 mvndep 2m 49s Maven dependency ordering for branch 0 mvndep 0m 14s Maven dependency ordering for patch +1 whitespace 0m 0s Patch has no whitespace issues. +1 asflicense 0m 44s Patch does not generate ASF License warnings. 4m 15s Subsystem Report/Notes Docker Image:yetus/hadoop:4bf84af JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798975/YARN-4514-YARN-3368.8.patch JIRA Issue YARN-4514 Optional Tests asflicense uname Linux 3ccea15df82d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-3368 / 4bf84af modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui . U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11108/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Committed to YARN-3368.

        Thanks Sunil G and thanks Varun Saxena for reviews!

        Show
        leftnoteasy Wangda Tan added a comment - Committed to YARN-3368 . Thanks Sunil G and thanks Varun Saxena for reviews!
        Hide
        sunilg Sunil G added a comment -

        Thank you Wangda Tan for the review and commit and thanks Varun Saxena for the review!

        Show
        sunilg Sunil G added a comment - Thank you Wangda Tan for the review and commit and thanks Varun Saxena for the review!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10778 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10778/)
        YARN-4514. YARN-3368 Cleanup hardcoded configurations, such as RM/ATS (wangda: rev dea4a296e558a11ba72e64344e8e34dcfba8598d)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-container-log.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/services/hosts.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/package.json
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node-app.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/services/env-test.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/cluster-metric.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-app.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/ember-cli-build.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-app-attempt.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-container.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/bower.json
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node-container.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/index.html
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/initializers/hosts.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/app.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/config.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/initializers/env.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/jquery-test.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-queue.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/default-config.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/cluster-info.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/services/hosts-test.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/abstract.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/hosts-test.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-rm-node.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/configs.env
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/env-test.js
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/services/env.js
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/environment.js
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10778 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10778/ ) YARN-4514 . YARN-3368 Cleanup hardcoded configurations, such as RM/ATS (wangda: rev dea4a296e558a11ba72e64344e8e34dcfba8598d) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-container-log.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/services/hosts.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/package.json (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node-app.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/services/env-test.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/cluster-metric.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-app.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/ember-cli-build.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-app-attempt.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-container.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/bower.json (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-node-container.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/index.html (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/initializers/hosts.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/app.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/config.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/initializers/env.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/jquery-test.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-queue.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/default-config.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/cluster-info.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/services/hosts-test.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/abstract.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/hosts-test.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/adapters/yarn-rm-node.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/configs.env (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/tests/unit/initializers/env-test.js (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/app/services/env.js (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-ui/src/main/webapp/config/environment.js

          People

          • Assignee:
            sunilg Sunil G
            Reporter:
            leftnoteasy Wangda Tan
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development