> On Jan. 26, 2017, 10:49 p.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/hdfs/consumer.md, line 92 > > <https://reviews.apache.org/r/52570/diff/3/?file=1616638#file1616638line92> > > > > nit: Use capitalizations consistently > > > > 1. Is `id` of any significance here?
Yes. it has to be exactly "[id]" > On Jan. 26, 2017, 10:49 p.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/jobs/configuration-table.html, line 1832 > > <https://reviews.apache.org/r/52570/diff/3/?file=1616641#file1616641line1832> > > > > Do we allow any other `partitioner` other than the defaultPartitioner? > > If not, it feels slightly unconventional to have `default` in the property > > name? This was discussed in the code review for implementation. And yes, we do allow other type of partitioner potentially in the future. That's why I was suggested to put "defaultPartitioner" here. > On Jan. 26, 2017, 10:49 p.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/jobs/configuration-table.html, line 1839 > > <https://reviews.apache.org/r/52570/diff/3/?file=1616641#file1616641line1839> > > > > Is the group identifier the partition name? > > Is `id` a reserved term? > > Do we always expect `[id]`? yes, it's a reserved term. updated the doc. > On Jan. 26, 2017, 10:49 p.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/jobs/configuration-table.html, line 1847 > > <https://reviews.apache.org/r/52570/diff/3/?file=1616641#file1616641line1847> > > > > The other minor observation is that the camel case naming scheme is > > slightly inconsistent with what we have for our public configs. > > > > For instance, job.coordinator. > > monitor-partition-change follows a different convention. I'd really > > prefer consistency in our config scheme. > > > > Navina or Prateek thoughts? I'm not sure what kind of discussion was going on there. But when I implemented the codes, I was explicitly told to adopt such a style (camel case). > On Jan. 26, 2017, 10:49 p.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/jobs/configuration-table.html, line 1842 > > <https://reviews.apache.org/r/52570/diff/3/?file=1616641#file1616641line1842> > > > > Is this the name of the class? I'm still not clear how this is used? It's literally "avro", or "plain", or "json". Though the last two are not supported now. No, they are not class name. - Hai ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52570/#review163177 ----------------------------------------------------------- On Jan. 26, 2017, 6:47 p.m., Hai Lu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52570/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 6:47 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-1025 > https://issues.apache.org/jira/browse/SAMZA-1025 > > > Repository: samza > > > Description > ------- > > documentation for hdfs system consumer > > > Diffs > ----- > > docs/learn/documentation/versioned/hdfs/consumer.md PRE-CREATION > docs/learn/documentation/versioned/hdfs/producer.md > b0e936f5b0a9c945ea7f02bfc2536ef50f017bf6 > docs/learn/documentation/versioned/index.html > d0b14ece94341e2cb937cf32db480e69f93303c2 > docs/learn/documentation/versioned/jobs/configuration-table.html > ba5ebbc54b5c64f82f35ed781dad7023a8f920e1 > > Diff: https://reviews.apache.org/r/52570/diff/ > > > Testing > ------- > > N/A > > > Thanks, > > Hai Lu > >
