> On Aug. 16, 2016, 12:50 a.m., Navina Ramesh wrote:
> >

Reminder: This review is for the doc, not the code. So any code feedback will 
not be reflected in this review. Instead see https://reviews.apache.org/r/50151/


> On Aug. 16, 2016, 12:50 a.m., Navina Ramesh wrote:
> > docs/learn/tutorials/versioned/samza-rest-getting-started.md, line 73
> > <https://reviews.apache.org/r/50154/diff/6/?file=1458034#file1458034line73>
> >
> >     Should change run-samza-rest-service.sh to be executable (chmod 755)

I think it is. I just applied the patch from JIRA to a fresh copy of samza on 
both my mac and linux machines an had no problems running through the tutorial.

Also:
```
samza-rest[master] > ls -la bin/run-samza-rest-service.sh 
-rwxr-xr-x  1  1.0K Aug 16 12:43 bin/run-samza-rest-service.sh*
```
and
```
samza-rest[master] > ls ../../../../src/main/bash/run-samza-rest-service.sh 
-rwxr-xr-x  1  1.0K Aug 16 12:43 
../../../../src/main/bash/run-samza-rest-service.sh*
```


> On Aug. 16, 2016, 12:50 a.m., Navina Ramesh wrote:
> > docs/learn/tutorials/versioned/samza-rest-getting-started.md, line 88
> > <https://reviews.apache.org/r/50154/diff/6/?file=1458034#file1458034line88>
> >
> >     I am running in to an issue of not finding config files in the path 
> > specified. I don't get any response on curl. 
> >     From the log file:
> >     
> >     2016-08-15 17:36:23.417 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/lib/config
> >     2016-08-15 17:36:23.417 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/bin/config
> >     2016-08-15 17:36:23.417 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/tmp/config
> >     2016-08-15 17:36:23.418 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/config/config
> >     2016-08-15 17:36:44.029 [qtp1276504061-19] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/lib/config
> >     2016-08-15 17:36:44.029 [qtp1276504061-19] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/bin/config
> >     2016-08-15 17:36:44.029 [qtp1276504061-19] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/tmp/config
> >     2016-08-15 17:36:44.030 [qtp1276504061-19] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/config/config
> >     2016-08-15 17:37:12.480 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/lib/config
> >     2016-08-15 17:37:12.481 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/bin/config
> >     2016-08-15 17:37:12.481 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/tmp/config
> >     2016-08-15 17:37:12.482 [qtp1276504061-18] SimpleInstallationFinder 
> > [WARN] Config path not found: 
> > /home/nramesh/Projects/samza-hello-samza/deploy/samza/config/config
> >     
> >     
> >     Looks like it is appening "config" to any path it tries to access. I 
> > have to look into your implementation code for why this is happening. Can 
> > you please check?

It's not really an "issue". It works fine, but the SimpleInstallationFinder 
wasn't written with ONLY hello-samza in mind and this log message is intended 
to help users determine why the JobsResource isn't finding their jobs. 

That said, I can see that it is clunky to always warn, so I'll change it to a 
DEBUG message in the other review.


> On Aug. 16, 2016, 12:50 a.m., Navina Ramesh wrote:
> > bin/generate-javadocs.sh, line 21
> > <https://reviews.apache.org/r/50154/diff/6/?file=1458025#file1458025line21>
> >
> >     We need to add this path to .gitignore.
> >     We should add this path to build.gradle excludes so that build will 
> > stop complaining about missing license headers on geenrated files.

Done, thanks


- Jake


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------


On Aug. 17, 2016, 1:45 a.m., Jake Maes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50154/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2016, 1:45 a.m.)
> 
> 
> Review request for samza, Boris Shkolnik, Chris Pettitt, Jake Maes, Navina 
> Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
> 
> 
> Bugs: SAMZA-976
>     https://issues.apache.org/jira/browse/SAMZA-976
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-976 Samza REST Documentation
> 
> 
> Diffs
> -----
> 
>   .gitignore 3e974168cc4f982151758b6727c69f17b9013f8c 
>   bin/generate-javadocs.sh da9f32da8f2a1f89ff28178ee703c4a065189da8 
>   docs/learn/documentation/versioned/index.html 
> 84e15a25bc8f1bb2996150bde24b81ff05dae224 
>   docs/learn/documentation/versioned/rest/monitors.md PRE-CREATION 
>   docs/learn/documentation/versioned/rest/overview.md PRE-CREATION 
>   docs/learn/documentation/versioned/rest/resource-directory.md PRE-CREATION 
>   docs/learn/documentation/versioned/rest/resources.md PRE-CREATION 
>   docs/learn/documentation/versioned/rest/resources/jobs.md PRE-CREATION 
>   docs/learn/tutorials/versioned/index.md 
> b4d687a63638aca4f876af88556de9973acfd718 
>   docs/learn/tutorials/versioned/samza-rest-getting-started.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50154/diff/
> 
> 
> Testing
> -------
> 
> Verified by building the local site documentation and browsing. I wasn't able 
> to verify the javadoc. If anyone knows why java doc would not be generated 
> for new classes, I'd be interested to know.
> 
> 
> Thanks,
> 
> Jake Maes
> 
>

Reply via email to