----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35397/#review88260 -----------------------------------------------------------
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 44) <https://reviews.apache.org/r/35397/#comment140682> change the scope? samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 74) <https://reviews.apache.org/r/35397/#comment140715> also canonicalize the classNames in the blacklistClassnames list? samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 75) <https://reviews.apache.org/r/35397/#comment140684> use "if" to be consistent? samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 115) <https://reviews.apache.org/r/35397/#comment140685> to be "if"? Just my $0.02 samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 156) <https://reviews.apache.org/r/35397/#comment140704> log here samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (lines 162 - 163) <https://reviews.apache.org/r/35397/#comment140705> doc may not be very precise. It's also possible that this class is blacklisted not ClassNotFoundException, right? I guess this is the reason you do not put line 164 in line 156 to simplify the code. samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 164) <https://reviews.apache.org/r/35397/#comment140706> you mean, parent.loadClass(name), right? samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala (lines 40 - 42) <https://reviews.apache.org/r/35397/#comment140717> add those to configuration table as well, including what kind of format we accept. samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala (line 112) <https://reviews.apache.org/r/35397/#comment140707> remove the space samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (lines 425 - 427) <https://reviews.apache.org/r/35397/#comment140708> do we want the TaskClassLoader to take care of the StreamTask itself? I thought we only used the TaskClassLoader to take care of what happens *inside* the StreamTask. samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala (lines 97 - 105) <https://reviews.apache.org/r/35397/#comment140709> a little concernted about this. This means we will load the class every time a message comes. Is this too much? My suggestion is to put this code in RunLoop.runs, before the loop starts. What do you think? samza-core/src/test/java/org/apache/samza/task/TestTaskClassLoader.java (line 88) <https://reviews.apache.org/r/35397/#comment140716> also want to test "org/example/foo" format - Yan Fang On June 16, 2015, 5:22 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35397/ > ----------------------------------------------------------- > > (Updated June 16, 2015, 5:22 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-697 > https://issues.apache.org/jira/browse/SAMZA-697 > > > Repository: samza > > > Description > ------- > > Use a separate post-delegate classloader for user-defined tasks > > > Diffs > ----- > > bin/check-all.sh 0725b82ed4f70b155f8bfe65cc6938d4647142d0 > docs/learn/documentation/versioned/jobs/logging.md > 1d13d151316bd51c7a3730e40450000433b7968e > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala > 0b3a235b5ab1d6bd60669bfe6023f6b0b4e943d3 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > cbacd183420e9d1d72b05693b55a8f0a62d59fc5 > samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala > c5a5ea5dea9a950fc741625238f5bf8b1f362180 > samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala > 1c178a661e449c6bdfc4ce431aef9bb2d261a6c2 > > samza-core/src/main/scala/org/apache/samza/job/local/ProcessJobFactory.scala > 4fac154709d72ab594485dad93c912b55fb1617e > samza-core/src/test/java/org/apache/samza/task/TestTaskClassLoader.java > PRE-CREATION > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala > 9fb1aa98fcd14397e8a4cb00c67537482e95fa53 > samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala > 7caad28c9298485753ab861da76793cf925953ed > > Diff: https://reviews.apache.org/r/35397/diff/ > > > Testing > ------- > > unit tests > > > Thanks, > > Guozhang Wang > >
