----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48393/#review137682 -----------------------------------------------------------
Overall lgtm. One additional high level comment: I thought that this would be a good time to refactor and move some of the scala files to java. Do you plan to do it later? P.S. let's discuss how to line up the check-ins of this one w/ potential 0.10.1 release. samza-core/src/main/java/org/apache/samza/clustermanager/AbstractContainerAllocator.java (line 158) <https://reviews.apache.org/r/48393/#comment202875> I thought that we discussed this earlier. Just want to confirm: why do we directly expose the member variables of SamzaApplicationState here? Won't it be better to encapsulate it? Was it because of the access privilege needed in index.scaml file? - Yi Pan (Data Infrastructure) On June 13, 2016, 11:59 p.m., Jagadish Venkatraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48393/ > ----------------------------------------------------------- > > (Updated June 13, 2016, 11:59 p.m.) > > > Review request for samza and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > ------- > > Integrate Kerberos with the refactored JC patch as in RB:47283. > > > Diffs > ----- > > > samza-core/src/main/java/org/apache/samza/clustermanager/AbstractContainerAllocator.java > 097a476bee75bb2b5a56ae85317fa16523baebd3 > > samza-core/src/main/java/org/apache/samza/clustermanager/ContainerProcessManager.java > f2db34c24b2b2ef05705a2fbf00d4a211d6f4c58 > > samza-core/src/main/java/org/apache/samza/clustermanager/SamzaApplicationState.java > ca277b3636aa4e98e639699e657de6aced613c0f > > samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala > 86c2440ad1d253fd71030916915984c1d08614fb > > samza-core/src/test/java/org/apache/samza/clustermanager/TestContainerAllocator.java > f1475709c2db364110b00cb04cf55c6ecb44fd04 > > samza-core/src/test/java/org/apache/samza/clustermanager/TestContainerProcessManager.java > 4fd1018f93dcfbccf70595c4bcdec956d2b59295 > > samza-core/src/test/java/org/apache/samza/clustermanager/TestHostAwareContainerAllocator.java > 57fef126de535067a68ce6eb8eeddf3334d1fe5d > samza-shell/src/main/bash/run-am.sh > ca938cc910b8b59e2cdab68df9f9ee16b425fb74 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/AbstractContainerAllocator.java > b4789e62beb1120f11a8101664b10c34ae930e58 > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerAllocator.java > 24ac41067e34cf5a445bb036db9ea324eaafa7df > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerFailure.java > 1d15651096e52a2e323a8e2f658fad3ea8e9c709 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerRequestState.java > 57ce35099973b7fc3414c450e3246cb9f204289b > samza-yarn/src/main/java/org/apache/samza/job/yarn/ContainerUtil.java > e8976bce934a324c48475bcd64d392119cc44b40 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/HostAwareContainerAllocator.java > 1d101fa80e6367d54f06455c242222195e4c0091 > samza-yarn/src/main/java/org/apache/samza/job/yarn/SamzaAppState.java > c116ed83f8b424bc7353e89c60cf165da542fbec > > samza-yarn/src/main/java/org/apache/samza/job/yarn/SamzaContainerRequest.java > 4a04eb6b3b054ee85988e1b26ececc800bbc7861 > samza-yarn/src/main/java/org/apache/samza/job/yarn/SamzaTaskManager.java > bc95f31c0dcaaa68d483a6f152b61aba6c543fff > samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnAppState.java > 57092e1b100b0128aed2a1426835d7184cc5bef7 > > samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java > 7778a380dcd99139012a71c80fb86ede1fef0f71 > samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml > 93660c73e507b1a6076e743fcec56a41d22a5f39 > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala > 1fb18bed9e6dfbbc45a1e277279d9513757d17bd > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterLifecycle.scala > 2a5c0d8092aa6f0a7f9b1f5fa56f9f4d4919d579 > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala > 054d8b68033535ff0cab7cca84b71455e201a715 > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterService.scala > 979d81d1b9871e0a1aa9e3c0fec7c13464392474 > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaYarnAppMasterLifecycle.scala > 2ed9baffc483d180aa6f5ff9e32abf9d1e1aef6f > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaYarnAppMasterService.scala > f62bec1b305b7301829644b5558eac197ea7457e > samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnJob.scala > 3ca1f0db59127d027604b015ddba07a550ceb114 > > samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala > a40ab72a71c4fa82bd87aa03caec8936e609bf68 > > samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterWebServlet.scala > 605332a02f365248fc9ec3525ed9d3721db71e8d > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocator.java > e21aded3c5e302593ed1cb5675da81aa6e3e743f > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerAllocatorCommon.java > 0bbd48d8720a79f3b453d69ca2772f0db10723be > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestContainerRequestState.java > 402fe784120ac40ed542d9fa60d6a6d7df9c8cda > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestHostAwareContainerAllocator.java > ead7200487f27ed31e30576721ef16a064a28bc7 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestSamzaContainerRequest.java > ad0f4d3d9db9111a7c0087b6da9c724dcc736726 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/TestSamzaTaskManager.java > d747b818ffdf0c9968afdf098b2a3f14b7b02a56 > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerAllocator.java > 329024729eeae07f8e755777b7211772cd98e04e > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerRequestState.java > 7c0b50437b06230ab840fdad894516c6d8545b4a > > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/MockContainerUtil.java > cf3e1439d005ca03f8c0e84183a26a0ca85abd0a > samza-yarn/src/test/java/org/apache/samza/job/yarn/util/TestUtil.java > d4c9c96144c58e1acc53726265a452f2467cd554 > samza-yarn/src/test/scala/org/apache/samza/job/yarn/MockSystemAdmin.scala > PRE-CREATION > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMaster.scala > 3f056c4d39fc8e3a2ed42fbcc73aa6eca1a2287c > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterLifecycle.scala > 750f467da12c5eb7ba80b06faadb1bac12feac1b > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterService.scala > 3de5614c56de9d0427fcfa22a864ab2df47aa095 > > Diff: https://reviews.apache.org/r/48393/diff/ > > > Testing > ------- > > Manual testing. > > Added Unit tests. > > Manually tested with a sample hello-world job. > > Tested that the UI was showing up correctly and template files were being > populated. f > > > Thanks, > > Jagadish Venkatraman > >
