You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Boris Shkolnik <bo...@apache.org> on 2016/09/22 00:48:53 UTC
Review Request 52140: added docs for split deployment
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/
-----------------------------------------------------------
Review request for samza.
Repository: samza
Description
-------
added docs for split deployment
Diffs
-----
docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
Diff: https://reviews.apache.org/r/52140/diff/
Testing
-------
Thanks,
Boris Shkolnik
Re: Review Request 52140: added docs for split deployment
Posted by Boris Shkolnik <bo...@apache.org>.
> On Sept. 22, 2016, 1:07 a.m., Xinyu Liu wrote:
> > docs/learn/documentation/versioned/operations/split-deployment.md, line 48
> > <https://reviews.apache.org/r/52140/diff/1/?file=1507623#file1507623line48>
> >
> > seems here are two "split deployment" bullets here. Do we want to distinguish the second and third type of deployments by naming?
well, they both are split deployment, and the difference is in the description. Otherwise we'll endup repeating parts of the description. I can add '1' and '2' but that is kind of useless.
> On Sept. 22, 2016, 1:07 a.m., Xinyu Liu wrote:
> > docs/learn/documentation/versioned/operations/split-deployment.md, line 65
> > <https://reviews.apache.org/r/52140/diff/1/?file=1507623#file1507623line65>
> >
> > 1. => 2.
this is md format. It will convert second '1' into '2', and third '1' into 3.
> On Sept. 22, 2016, 1:07 a.m., Xinyu Liu wrote:
> > docs/learn/documentation/versioned/operations/split-deployment.md, line 90
> > <https://reviews.apache.org/r/52140/diff/1/?file=1507623#file1507623line90>
> >
> > Since this is 0.11 release, can we use 0.11.0 acrose all examples here?
Sure.
> On Sept. 22, 2016, 1:07 a.m., Xinyu Liu wrote:
> > docs/learn/documentation/versioned/operations/split-deployment.md, line 113
> > <https://reviews.apache.org/r/52140/diff/1/?file=1507623#file1507623line113>
> >
> > I think we need to mention the framework libraries needed to be placed to each node in the cluster.
we are talking about 206 jars! how do you mention it here? I guess the user should figure it out by themselves (for example copy from a current deployment).
- Boris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/#review149931
-----------------------------------------------------------
On Sept. 22, 2016, 12:48 a.m., Boris Shkolnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52140/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2016, 12:48 a.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> added docs for split deployment
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52140/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Boris Shkolnik
>
>
Re: Review Request 52140: added docs for split deployment
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/#review149931
-----------------------------------------------------------
Fix it, then Ship it!
Some minor suggestions.
docs/learn/documentation/versioned/operations/split-deployment.md (line 36)
<https://reviews.apache.org/r/52140/#comment217708>
is this <br> intended here?
docs/learn/documentation/versioned/operations/split-deployment.md (line 48)
<https://reviews.apache.org/r/52140/#comment217709>
seems here are two "split deployment" bullets here. Do we want to distinguish the second and third type of deployments by naming?
docs/learn/documentation/versioned/operations/split-deployment.md (line 65)
<https://reviews.apache.org/r/52140/#comment217711>
1. => 2.
docs/learn/documentation/versioned/operations/split-deployment.md (line 66)
<https://reviews.apache.org/r/52140/#comment217712>
3. ..
docs/learn/documentation/versioned/operations/split-deployment.md (line 90)
<https://reviews.apache.org/r/52140/#comment217714>
Since this is 0.11 release, can we use 0.11.0 acrose all examples here?
docs/learn/documentation/versioned/operations/split-deployment.md (line 113)
<https://reviews.apache.org/r/52140/#comment217715>
I think we need to mention the framework libraries needed to be placed to each node in the cluster.
- Xinyu Liu
On Sept. 22, 2016, 12:48 a.m., Boris Shkolnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52140/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2016, 12:48 a.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> added docs for split deployment
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52140/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Boris Shkolnik
>
>
Re: Review Request 52140: added docs for split deployment
Posted by Jagadish Venkatraman <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/#review150104
-----------------------------------------------------------
docs/learn/documentation/versioned/operations/split-deployment.md (line 44)
<https://reviews.apache.org/r/52140/#comment217956>
If it's not addressed here, I wonder if we should specify this in this doc? Let me know what you think.
docs/learn/documentation/versioned/operations/split-deployment.md (line 46)
<https://reviews.apache.org/r/52140/#comment217957>
Why is it `possibly` in the `future`? Seems like a lot of `ifs` ;-)
Can't they do it currently by configuring these properties?
docs/learn/documentation/versioned/operations/split-deployment.md (line 64)
<https://reviews.apache.org/r/52140/#comment217958>
why are all line numberings `1`. Is this a typo or some `md` format I don't understand?
docs/learn/documentation/versioned/operations/split-deployment.md (line 65)
<https://reviews.apache.org/r/52140/#comment217959>
With the refactored code path, is this still the `SamzaAppMaster` class?
docs/learn/documentation/versioned/operations/split-deployment.md (line 92)
<https://reviews.apache.org/r/52140/#comment217966>
Should the user do anything to `add` this to the classpath? It was not clear from the doc.
nit: typo in bellow
s/bellow/below
docs/learn/documentation/versioned/operations/split-deployment.md (line 99)
<https://reviews.apache.org/r/52140/#comment217963>
nit-alert:
/s/bellow/below
docs/learn/documentation/versioned/operations/split-deployment.md (line 103)
<https://reviews.apache.org/r/52140/#comment217964>
Maybe I missed something - What do you mean by path to the containers? Did you mean path to the framework instead?
docs/learn/documentation/versioned/operations/split-deployment.md (line 105)
<https://reviews.apache.org/r/52140/#comment217965>
It was not clear to me - what do you meant by old way? How about re-wording this like - `this will fall-back to the default way`?
docs/learn/documentation/versioned/operations/split-deployment.md (line 120)
<https://reviews.apache.org/r/52140/#comment217970>
For consistency, we should capitalize `samza`.
s/samza/Samza everywhere. I've seen this being consistently done in the other parts of the open source docs like http://samza.apache.org/learn/documentation/0.10/container/samza-container.html
docs/learn/documentation/versioned/operations/split-deployment.md (line 122)
<https://reviews.apache.org/r/52140/#comment217967>
nit:
typo here..
s/SMAZA/SAMZA
docs/learn/documentation/versioned/operations/split-deployment.md (line 123)
<https://reviews.apache.org/r/52140/#comment217968>
nit:
If the setting is empty`,`.. What `setting` are you referring to?(it should be `$VERSION`)..
docs/learn/documentation/versioned/operations/split-deployment.md (line 124)
<https://reviews.apache.org/r/52140/#comment217969>
s/lib path / library path
- Jagadish Venkatraman
On Sept. 22, 2016, 11:43 p.m., Boris Shkolnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52140/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2016, 11:43 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-927
> https://issues.apache.org/jira/browse/SAMZA-927
>
>
> Repository: samza
>
>
> Description
> -------
>
> added docs for split deployment
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52140/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Boris Shkolnik
>
>
Re: Review Request 52140: added docs for split deployment
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/#review150500
-----------------------------------------------------------
Ship it!
Ship It!
- Xinyu Liu
On Sept. 27, 2016, 12:58 a.m., Boris Shkolnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52140/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2016, 12:58 a.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-927
> https://issues.apache.org/jira/browse/SAMZA-927
>
>
> Repository: samza
>
>
> Description
> -------
>
> added docs for split deployment
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/index.html a997445db2af40e3ac87dc01d510a5e9045d73f4
> docs/learn/documentation/versioned/jobs/split-deployment.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52140/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Boris Shkolnik
>
>
Re: Review Request 52140: added docs for split deployment
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/
-----------------------------------------------------------
(Updated Sept. 27, 2016, 12:58 a.m.)
Review request for samza.
Changes
-------
converted document to be more end-user oriented.
Bugs: SAMZA-927
https://issues.apache.org/jira/browse/SAMZA-927
Repository: samza
Description
-------
added docs for split deployment
Diffs (updated)
-----
docs/learn/documentation/versioned/index.html a997445db2af40e3ac87dc01d510a5e9045d73f4
docs/learn/documentation/versioned/jobs/split-deployment.md PRE-CREATION
Diff: https://reviews.apache.org/r/52140/diff/
Testing
-------
Thanks,
Boris Shkolnik
Re: Review Request 52140: added docs for split deployment
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/
-----------------------------------------------------------
(Updated Sept. 23, 2016, 5:24 p.m.)
Review request for samza.
Bugs: SAMZA-927
https://issues.apache.org/jira/browse/SAMZA-927
Repository: samza
Description
-------
added docs for split deployment
Diffs (updated)
-----
docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
Diff: https://reviews.apache.org/r/52140/diff/
Testing
-------
Thanks,
Boris Shkolnik
Re: Review Request 52140: added docs for split deployment
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/
-----------------------------------------------------------
(Updated Sept. 22, 2016, 11:43 p.m.)
Review request for samza.
Bugs: SAMZA-927
https://issues.apache.org/jira/browse/SAMZA-927
Repository: samza
Description
-------
added docs for split deployment
Diffs
-----
docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
Diff: https://reviews.apache.org/r/52140/diff/
Testing
-------
Thanks,
Boris Shkolnik
Re: Review Request 52140: added docs for split deployment
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/#review150101
-----------------------------------------------------------
Ship it!
Ship It!
- Xinyu Liu
On Sept. 22, 2016, 11:38 p.m., Boris Shkolnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52140/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2016, 11:38 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> added docs for split deployment
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52140/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Boris Shkolnik
>
>
Re: Review Request 52140: added docs for split deployment
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52140/
-----------------------------------------------------------
(Updated Sept. 22, 2016, 11:38 p.m.)
Review request for samza.
Repository: samza
Description
-------
added docs for split deployment
Diffs (updated)
-----
docs/learn/documentation/versioned/operations/split-deployment.md PRE-CREATION
Diff: https://reviews.apache.org/r/52140/diff/
Testing
-------
Thanks,
Boris Shkolnik