Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44836]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 16, 2016, 2:27 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 16, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Klaus Ma]

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




src/examples/long_lived_framework.cpp (line 88)


I think we need check `format()`'s result; if `tasksLaunched` is bigger 
than `99`, what's the behaviour?


- Klaus Ma


On March 16, 2016, 10:27 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 16, 2016, 10:27 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

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

(Updated March 16, 2016, 2:27 a.m.)


Review request for mesos and haosdent huang.


Changes
---

rebase master


Repository: mesos


Description
---

Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
This patch updates long_lived_framework to use `01`, `02` instead.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 

Diff: https://reviews.apache.org/r/44836/diff/


Testing
---

make check

In addition, `long_lived_framework` is ran and we confirm correct sorting of 
`TaskID` in WebUI


Thanks,

Jay Guo

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

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

(Updated March 16, 2016, 2:21 a.m.)


Review request for mesos and haosdent huang.


Changes
---

remove unused `#include`


Repository: mesos


Description
---

Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
This patch updates long_lived_framework to use `01`, `02` instead.


Diffs (updated)
-

  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 
  src/exec/exec.cpp 741786132f3a8cc43f5b9ced262429038832a946 
  src/executor/executor.cpp 87db4e02cbaa778aab0173741bfe066fdee9a48d 
  src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
  src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 

Diff: https://reviews.apache.org/r/44836/diff/


Testing
---

make check

In addition, `long_lived_framework` is ran and we confirm correct sorting of 
`TaskID` in WebUI


Thanks,

Jay Guo

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44836]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 15, 2016, 7:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 7:49 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[haosdent huang]

> On March 15, 2016, 7:04 a.m., haosdent huang wrote:
> >
> 
> haosdent huang wrote:
> I also notice the submitter `guojian...@cn.ibm.com` is different with 
> signer `Zhou Xing `. I suggest don't do it like this.
> 
> Jay Guo wrote:
> We are following pair programming pattern and use git-duet. Will do clean 
> commit in the future. Also, IMHO, sorting task ID in WebUI by detecting 
> string type would be a more appropriate solution, instead of forcing 
> framework to use string IDs. Maybe we could bring this up at a later point.

Agree with you.


- haosdent


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


On March 15, 2016, 7:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 7:49 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

> On March 15, 2016, 7:04 a.m., haosdent huang wrote:
> >
> 
> haosdent huang wrote:
> I also notice the submitter `guojian...@cn.ibm.com` is different with 
> signer `Zhou Xing `. I suggest don't do it like this.

We are following pair programming pattern and use git-duet. Will do clean 
commit in the future. Also, IMHO, sorting task ID in WebUI by detecting string 
type would be a more appropriate solution, instead of forcing framework to use 
string IDs. Maybe we could bring this up at a later point.


- Jay


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


On March 15, 2016, 7:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 7:49 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

> On March 15, 2016, 7:01 a.m., haosdent huang wrote:
> > Thank you for your patch. Could you update the `Bugs` field to 
> > `MESOS-4930`? By the way, I think you need find a shepherd for this ticket. 
> > Could you follow this guide 
> > https://github.com/apache/mesos/blob/master/docs/submitting-a-patch.md to 
> > find the committer which could help you review this more detail and help 
> > you submit?

Thanks for reviewing. We've sent a mail to dev mailing list to look for 
shepherd.


- Jay


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


On March 15, 2016, 7:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 7:49 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[haosdent huang]

> On March 15, 2016, 7:04 a.m., haosdent huang wrote:
> >

I also notice the submitter `guojian...@cn.ibm.com` is different with signer 
`Zhou Xing `. I suggest don't do it like this.


- haosdent


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


On March 15, 2016, 6:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> Signed-off-by: Zhou Xing 
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[haosdent huang]

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




src/examples/long_lived_framework.cpp (line 98)


Need remove cast if we change taskId to string.



src/examples/long_lived_framework.cpp (line 99)


Ditto



src/examples/long_lived_framework.cpp (line 130)


Also need update here if the type of taskId is string.


- haosdent huang


On March 15, 2016, 6:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> Signed-off-by: Zhou Xing 
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[haosdent huang]

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



Thank you for your patch. Could you update the `Bugs` field to `MESOS-4930`? By 
the way, I think you need find a shepherd for this ticket. Could you follow 
this guide 
https://github.com/apache/mesos/blob/master/docs/submitting-a-patch.md to find 
the committer which could help you review this more detail and help you submit?


src/examples/long_lived_framework.cpp (line 92)


We have a function in ``. I think you could use it like 
this directly.

```
strings::format("%06d", xxx)
```


- haosdent huang


On March 15, 2016, 6:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44836/
> ---
> 
> (Updated March 15, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
> This patch updates long_lived_framework to use `01`, `02` instead.
> 
> Signed-off-by: Zhou Xing 
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/44836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> In addition, `long_lived_framework` is ran and we confirm correct sorting of 
> `TaskID` in WebUI
> 
> 
> Thanks,
> 
> Jay Guo
> 
>