Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60124, 60123, 60122]

Failed command: python support/apply-reviews.py -n -r 60124

Error:
error: patch failed: src/examples/balloon_framework.cpp:494
error: src/examples/balloon_framework.cpp: patch does not apply

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/112/console

- Mesos Reviewbot Windows


On June 15, 2017, 7:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 7:09 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Alexander Rukletsov]

> On June 15, 2017, 7:07 p.m., Armand Grillet wrote:
> > src/examples/balloon_framework.cpp
> > Line 464 (original), 469 (patched)
> > 
> >
> > Consistency wise, other tests have just `executor.set_name("Framework 
> > Name Executor (C++)");` to set the name of the executor (e.g. in 
> > `long_lived_framework.cpp`). More generally, what is the reason to add a 
> > `name` flag for this test?
> 
> Alexander Rukletsov wrote:
> If we have multiple instance of the same framework, it can be difficult 
> to distinguish between them in the WebUI, hence this change. I agree, this is 
> not only relevant to this framework, but actually to all example frameworks.
> 
> Joseph Wu wrote:
> You might as well let the flag control the entire name, rather than 
> appending `--name` to the end of `Balloon Framework`.  Or update the flag 
> description to mention that it will be appended to the canonical framework 
> name.

Good idea, Joseph.


- Alexander


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


On June 15, 2017, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 5:09 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Joseph Wu]

> On June 15, 2017, 12:07 p.m., Armand Grillet wrote:
> > src/examples/balloon_framework.cpp
> > Line 464 (original), 469 (patched)
> > 
> >
> > Consistency wise, other tests have just `executor.set_name("Framework 
> > Name Executor (C++)");` to set the name of the executor (e.g. in 
> > `long_lived_framework.cpp`). More generally, what is the reason to add a 
> > `name` flag for this test?
> 
> Alexander Rukletsov wrote:
> If we have multiple instance of the same framework, it can be difficult 
> to distinguish between them in the WebUI, hence this change. I agree, this is 
> not only relevant to this framework, but actually to all example frameworks.

You might as well let the flag control the entire name, rather than appending 
`--name` to the end of `Balloon Framework`.  Or update the flag description to 
mention that it will be appended to the canonical framework name.


- Joseph


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


On June 15, 2017, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 10:09 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Alexander Rukletsov]

> On June 15, 2017, 7:07 p.m., Armand Grillet wrote:
> > src/examples/balloon_framework.cpp
> > Line 464 (original), 469 (patched)
> > 
> >
> > Consistency wise, other tests have just `executor.set_name("Framework 
> > Name Executor (C++)");` to set the name of the executor (e.g. in 
> > `long_lived_framework.cpp`). More generally, what is the reason to add a 
> > `name` flag for this test?

If we have multiple instance of the same framework, it can be difficult to 
distinguish between them in the WebUI, hence this change. I agree, this is not 
only relevant to this framework, but actually to all example frameworks.


- Alexander


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


On June 15, 2017, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 5:09 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Armand Grillet]

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




src/examples/balloon_framework.cpp
Line 464 (original), 469 (patched)


Consistency wise, other tests have just `executor.set_name("Framework Name 
Executor (C++)");` to set the name of the executor (e.g. in 
`long_lived_framework.cpp`). More generally, what is the reason to add a `name` 
flag for this test?


- Armand Grillet


On June 15, 2017, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 5:09 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Alexander Rukletsov]

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

Review request for mesos, Armand Grillet and Joseph Wu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 


Diff: https://reviews.apache.org/r/60124/diff/1/


Testing
---

make check


Thanks,

Alexander Rukletsov