You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ross Allen <ro...@mesosphe.re> on 2013/10/15 00:38:27 UTC

Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

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

Review request for mesos.


Repository: mesos-git


Description
-------

Truncated only Mesos IDs that look like UUIDs.

Framework IDs are truncated regardless of their format right now, but
frameworks assign their own IDs without any restrictions. For frameworks
that use IDs that don't look like UUIDs, they end up with just empty
strings as their truncated IDs.


Diffs
-----

  src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 

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


Testing
-------


Thanks,

Ross Allen

Re: Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

Posted by Ross Allen <ro...@mesosphe.re>.

> On Oct. 15, 2013, 3 a.m., Ben Mahler wrote:
> > It is not supposed to be the case that frameworks have a custom ID, although it is possible to trick the Master by providing a custom ID when creating a SchedulerDriver. This causes the scheduler driver to treat the framework as having failed over. Ideally we would guard against this behavior but it requires persistent framework state in the Master to do correctly.
> > 
> > If you look at the framework registration code in master.cpp you will notice newFrameworkId() which is how these IDs are intended to be assigned. Schedulers are then expected to use this ID when subsequently constructing a SchedulerDriver.
> > 
> > Do you know of frameworks that are not following the API in this way? Either way we should improve the scheduler.hpp documentation and ideally guard against this in the future!

It still seems odd that the truncation happens without first inspecting the string. Since there's no enforcement of the ID format in master, it seems reasonable to truncate in the UI only if it the string should be truncated.


- Ross


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


On Oct. 14, 2013, 10:38 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14635/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2013, 10:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Truncated only Mesos IDs that look like UUIDs.
> 
> Framework IDs are truncated regardless of their format right now, but
> frameworks assign their own IDs without any restrictions. For frameworks
> that use IDs that don't look like UUIDs, they end up with just empty
> strings as their truncated IDs.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 
> 
> Diff: https://reviews.apache.org/r/14635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>

Re: Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 15, 2013, 3 a.m., Ben Mahler wrote:
> > It is not supposed to be the case that frameworks have a custom ID, although it is possible to trick the Master by providing a custom ID when creating a SchedulerDriver. This causes the scheduler driver to treat the framework as having failed over. Ideally we would guard against this behavior but it requires persistent framework state in the Master to do correctly.
> > 
> > If you look at the framework registration code in master.cpp you will notice newFrameworkId() which is how these IDs are intended to be assigned. Schedulers are then expected to use this ID when subsequently constructing a SchedulerDriver.
> > 
> > Do you know of frameworks that are not following the API in this way? Either way we should improve the scheduler.hpp documentation and ideally guard against this in the future!
> 
> Ross Allen wrote:
>     It still seems odd that the truncation happens without first inspecting the string. Since there's no enforcement of the ID format in master, it seems reasonable to truncate in the UI only if it the string should be truncated.

Good point!


- Ben


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


On Oct. 14, 2013, 10:38 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14635/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2013, 10:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Truncated only Mesos IDs that look like UUIDs.
> 
> Framework IDs are truncated regardless of their format right now, but
> frameworks assign their own IDs without any restrictions. For frameworks
> that use IDs that don't look like UUIDs, they end up with just empty
> strings as their truncated IDs.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 
> 
> Diff: https://reviews.apache.org/r/14635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>

Re: Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14635/#review27003
-----------------------------------------------------------


It is not supposed to be the case that frameworks have a custom ID, although it is possible to trick the Master by providing a custom ID when creating a SchedulerDriver. This causes the scheduler driver to treat the framework as having failed over. Ideally we would guard against this behavior but it requires persistent framework state in the Master to do correctly.

If you look at the framework registration code in master.cpp you will notice newFrameworkId() which is how these IDs are intended to be assigned. Schedulers are then expected to use this ID when subsequently constructing a SchedulerDriver.

Do you know of frameworks that are not following the API in this way? Either way we should improve the scheduler.hpp documentation and ideally guard against this in the future!

- Ben Mahler


On Oct. 14, 2013, 10:38 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14635/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2013, 10:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Truncated only Mesos IDs that look like UUIDs.
> 
> Framework IDs are truncated regardless of their format right now, but
> frameworks assign their own IDs without any restrictions. For frameworks
> that use IDs that don't look like UUIDs, they end up with just empty
> strings as their truncated IDs.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 
> 
> Diff: https://reviews.apache.org/r/14635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>

Re: Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

Posted by Ross Allen <ro...@mesosphe.re>.

> On Oct. 15, 2013, 6:05 p.m., Ben Mahler wrote:
> > src/webui/master/static/js/app.js, line 51
> > <https://reviews.apache.org/r/14635/diff/1/?file=364617#file364617line51>
> >
> >     Should this be:
> >     
> >     truncatedIdParts.splice(3, Number.MAX_VALUE)?
> >     
> >

Actually, if this is meant to select everything beyond the 3rd index, `slice` is the more appropriate call.

    truncatedIdParts.slice(3).join('-')

I didn't see your comment until after I pushed to master, so I'll send another request to change it.


- Ross


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


On Oct. 14, 2013, 10:38 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14635/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2013, 10:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Truncated only Mesos IDs that look like UUIDs.
> 
> Framework IDs are truncated regardless of their format right now, but
> frameworks assign their own IDs without any restrictions. For frameworks
> that use IDs that don't look like UUIDs, they end up with just empty
> strings as their truncated IDs.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 
> 
> Diff: https://reviews.apache.org/r/14635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>

Re: Review Request 14635: Truncated only Mesos IDs that look like UUIDs.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14635/#review27035
-----------------------------------------------------------

Ship it!



src/webui/master/static/js/app.js
<https://reviews.apache.org/r/14635/#comment52639>

    Should this be:
    
    truncatedIdParts.splice(3, Number.MAX_VALUE)?
    
    


- Ben Mahler


On Oct. 14, 2013, 10:38 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14635/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2013, 10:38 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Truncated only Mesos IDs that look like UUIDs.
> 
> Framework IDs are truncated regardless of their format right now, but
> frameworks assign their own IDs without any restrictions. For frameworks
> that use IDs that don't look like UUIDs, they end up with just empty
> strings as their truncated IDs.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 92e8e2dd36091f109be3cf3ce26fd23f558e6890 
> 
> Diff: https://reviews.apache.org/r/14635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>