You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Navina Ramesh <nr...@linkedin.com> on 2016/08/16 00:50:08 UTC
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------
bin/generate-javadocs.sh (line 21)
<https://reviews.apache.org/r/50154/#comment212142>
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.
docs/learn/tutorials/versioned/samza-rest-getting-started.md (line 22)
<https://reviews.apache.org/r/50154/#comment210303>
nit: "executed a couple of basic curl ..."
docs/learn/tutorials/versioned/samza-rest-getting-started.md (line 73)
<https://reviews.apache.org/r/50154/#comment212172>
Should change run-samza-rest-service.sh to be executable (chmod 755)
docs/learn/tutorials/versioned/samza-rest-getting-started.md (line 88)
<https://reviews.apache.org/r/50154/#comment212177>
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?
- Navina Ramesh
On July 29, 2016, 6:58 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50154/
> -----------------------------------------------------------
>
> (Updated July 29, 2016, 6:58 p.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
> -----
>
> bin/generate-javadocs.sh da9f32da8f2a1f89ff28178ee703c4a065189da8
> docs/img/versioned/learn/documentation/rest/JobsResource.png PRE-CREATION
> docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23
> 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
>
>
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
Posted by Jake Maes <ja...@gmail.com>.
> 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?
>
> Jake Maes wrote:
> 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.
>
> Navina Ramesh wrote:
> 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.
> > Ok. let me try to understand this. This code is recursively searching the installation path for a config file. Is that what is happening here and is that why there were so many log lines?
> Anyway, my point with this comments was that I was not getting the expected output from the curl command. With your latest patch, looks like it got fixed. Thanks!
>
> Jake Maes wrote:
> Also turns out there was an issue with the instructions. The config should point to the deploy/ directory, not deploy/samza. I fixed that too. Thanks!
Right, I missed that comment initially and focused on the log message.
The issue was with the instructions setting the wrong path in the properties file. Thanks for finding it!
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------
On Aug. 17, 2016, 1:58 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:58 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
>
>
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
Posted by Jake Maes <ja...@gmail.com>.
> 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?
>
> Jake Maes wrote:
> 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.
>
> Navina Ramesh wrote:
> 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.
> > Ok. let me try to understand this. This code is recursively searching the installation path for a config file. Is that what is happening here and is that why there were so many log lines?
> Anyway, my point with this comments was that I was not getting the expected output from the curl command. With your latest patch, looks like it got fixed. Thanks!
Also turns out there was an issue with the instructions. The config should point to the deploy/ directory, not deploy/samza. I fixed that too. Thanks!
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------
On Aug. 17, 2016, 1:58 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:58 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
>
>
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
Posted by Jake Maes <ja...@gmail.com>.
> 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
>
>
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
Posted by Jake Maes <ja...@gmail.com>.
> 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)
>
> Jake Maes wrote:
> 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*
> ```
>
> Navina Ramesh wrote:
> Weird.. even with your new patch it still complained about permissions. Never mind. I will verify it and fix it, if required before pushing it. Thanks!
Hmm, maybe its a difference between ```git apply``` and ```patch``` commands. Thanks for making sure it works. And remember to also include the PNG file attached to the JIRA. The path is specified in the JIRA comments.
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------
On Aug. 17, 2016, 1:58 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:58 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
>
>
Re: Review Request 50154: SAMZA-976 Samza REST Documentation
Posted by Navina Ramesh <nr...@linkedin.com>.
> 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)
>
> Jake Maes wrote:
> 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*
> ```
Weird.. even with your new patch it still complained about permissions. Never mind. I will verify it and fix it, if required before pushing it. Thanks!
> 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?
>
> Jake Maes wrote:
> 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.
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.
> Ok. let me try to understand this. This code is recursively searching the installation path for a config file. Is that what is happening here and is that why there were so many log lines?
Anyway, my point with this comments was that I was not getting the expected output from the curl command. With your latest patch, looks like it got fixed. Thanks!
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50154/#review144296
-----------------------------------------------------------
On Aug. 17, 2016, 1:58 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:58 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
>
>