----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23564/#review48026 -----------------------------------------------------------
I think my prior comments were dropped, since I RB'd twice. The latest patch seems to just introduce the isCompressed boolean. Attaching the originals again. samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala <https://reviews.apache.org/r/23564/#comment84273> In keeping with existing config style, can we do task.config.compress? samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala <https://reviews.apache.org/r/23564/#comment84272> Having a var named paramVal seems kind of odd. I think there is a slightly more idiomatic way of doing this. Simply have the if block return either Util.decompress(param)'s results as the last line, or return param itself. val isCompressed = System.getenv(ShellCommandConfig.ENV_COMPRESS_CONFIG) if (isCompressed.equals("TRUE")) { info("Compression is ON !") val decommpressedParam = Util.decompress(param) info("Got param = " + decommpressedParam) decommpressedParam } else { info("Parameter is uncompressed. Using it as-is") param } - Chris Riccomini On July 17, 2014, 5:46 p.m., Chinmay Soman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23564/ > ----------------------------------------------------------- > > (Updated July 17, 2014, 5:46 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-337 > https://issues.apache.org/jira/browse/SAMZA-337 > > > Repository: samza > > > Description > ------- > > SAMZA-337: More review comments > > > SAMZA-337: Addressing review comments > > > SAMZA-337: Compress Samza configuration passed to Yarn > > > Diffs > ----- > > build.gradle 6d0ac973394bb0d733179ef954a973a06d024aee > gradle/dependency-versions.gradle 73735821426d5427cfbcdcb5fd86d8fe7ac53447 > samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala > 4c2d365705bc21070dba8f2b889bcb68faccc962 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > b303615ca0978a92febe227fcf88b5acecce4223 > samza-core/src/main/scala/org/apache/samza/job/ShellCommandBuilder.scala > f8865b19b9afd3cab4ca7bbed4567ab800c4a648 > samza-core/src/main/scala/org/apache/samza/util/Util.scala > 11c23d0fdf2cd9daaac5b8e248acc51464d64d06 > samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala > a67ecdf44af1a35d88b3d32dd2a05861bf728440 > > Diff: https://reviews.apache.org/r/23564/diff/ > > > Testing > ------- > > Tested by adding this property to all the jobs in hello-samza: > > +config.compress=true > > After deploying the job, I see a compressed value in launch_container.sh. The > job ran successfully in this case (also tested with config.compress=false) > > > Thanks, > > Chinmay Soman > >
