You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Brenden Matthews <br...@diddyinc.com> on 2013/10/22 21:04:42 UTC

Re: Review Request 11108: Simplified os::hostname() in libstout.

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

(Updated Oct. 22, 2013, 7:04 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

I can't update the diff for this:

brenden@airbob ~/dev/mesos $ git cherry-pick f7f03b4a7adbe8cff28459076abc03ca552d0bf4
[am 9483529] Simplified os::hostname() in libstout.
 1 file changed, 2 insertions(+), 31 deletions(-)
brenden@airbob ~/dev/mesos $ ./support/post-reviews.py --target-groups=mesos --guess-fields
Running post-review across all of ...
948352938d3c25f346a78fe042ab56df12373cc5 - (HEAD, am) Simplified os::hostname() in libstout. (2 seconds ago)
Updating diff of:

948352938d3c25f346a78fe042ab56df12373cc5 - (HEAD, am) Simplified os::hostname() in libstout. (2 seconds ago)
... with parent diff created from:


Press enter to continue or 'Ctrl-C' to skip.

Failed to execute: 'post-review --review-request-id=11108 --repository-url=git://git.apache.org/mesos.git --tracking-branch=master --revision-range=master:948352938d3c25f346a78fe042ab56df12373cc5 --target-groups=mesos --guess-fields':

Error uploading diff

The generated diff file was empty. This usually means no files were
modified in this change.

Try running with --output-diff and --debug for more information.

Your review request still exists, but the diff is not attached.

brenden@airbob ~/dev/mesos $ 


The code is available here though:

https://github.com/airbnb/mesos/commit/f7f03b4a7adbe8cff28459076abc03ca552d0bf4


Summary (updated)
-----------------

Simplified os::hostname() in libstout.


Repository: mesos


Description (updated)
-------

Simplified os::hostname() in libstout.

Review: https://reviews.apache.org/r/11108


Diffs
-----

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 

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


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11108: Simplified os::hostname() in libstout.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11108/#review87249
-----------------------------------------------------------


Bad patch!

Reviews applied: [11108]

Failed command: ./support/apply-review.sh -n -r 11108

Error:
 2015-06-09 18:16:26 URL:https://reviews.apache.org/r/11108/diff/raw/ [1691/1691] -> "11108.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:52
error: 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 22, 2013, 7:04 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11108/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 7:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> -------
> 
> Simplified os::hostname() in libstout.
> 
> Review: https://reviews.apache.org/r/11108
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
> 
> Diff: https://reviews.apache.org/r/11108/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11108: Simplified os::hostname() in libstout.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11108/#review87241
-----------------------------------------------------------


@brenden: Still want this in? If not, let's close it for now.

- Niklas Nielsen


On Oct. 22, 2013, 12:04 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11108/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 12:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> -------
> 
> Simplified os::hostname() in libstout.
> 
> Review: https://reviews.apache.org/r/11108
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
> 
> Diff: https://reviews.apache.org/r/11108/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11108: Simplified os::hostname() in libstout.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Oct. 22, 2013, 7:54 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 820
> > <https://reviews.apache.org/r/11108/diff/6/?file=303652#file303652line820>
> >
> >     This code was trying to get the hostname as described here: http://stackoverflow.com/questions/504810/how-do-i-find-the-current-machines-full-hostname-in-c-hostname-and-domain-info/505546#505546. IIUC the double lookup enables us to map a potential 'localhost' (returned from gethostname) into a 'foo.bar.baz'. What seemed to fail for you?

For us, this would return the reverse DNS of the internal IP.  This is an address that isn't accessible externally.


- Brenden


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


On Oct. 22, 2013, 7:04 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11108/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 7:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Simplified os::hostname() in libstout.
> 
> Review: https://reviews.apache.org/r/11108
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
> 
> Diff: https://reviews.apache.org/r/11108/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11108: Simplified os::hostname() in libstout.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 22, 2013, 7:54 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 820
> > <https://reviews.apache.org/r/11108/diff/6/?file=303652#file303652line820>
> >
> >     This code was trying to get the hostname as described here: http://stackoverflow.com/questions/504810/how-do-i-find-the-current-machines-full-hostname-in-c-hostname-and-domain-info/505546#505546. IIUC the double lookup enables us to map a potential 'localhost' (returned from gethostname) into a 'foo.bar.baz'. What seemed to fail for you?
> 
> Brenden Matthews wrote:
>     For us, this would return the reverse DNS of the internal IP.  This is an address that isn't accessible externally.

So, just to be clear, your fix causes os::hostname to return the public hostname?


- Benjamin


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


On Oct. 22, 2013, 7:04 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11108/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 7:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Simplified os::hostname() in libstout.
> 
> Review: https://reviews.apache.org/r/11108
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
> 
> Diff: https://reviews.apache.org/r/11108/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11108: Simplified os::hostname() in libstout.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11108/#review27330
-----------------------------------------------------------



3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/11108/#comment53158>

    This code was trying to get the hostname as described here: http://stackoverflow.com/questions/504810/how-do-i-find-the-current-machines-full-hostname-in-c-hostname-and-domain-info/505546#505546. IIUC the double lookup enables us to map a potential 'localhost' (returned from gethostname) into a 'foo.bar.baz'. What seemed to fail for you?


- Benjamin Hindman


On Oct. 22, 2013, 7:04 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11108/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2013, 7:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Simplified os::hostname() in libstout.
> 
> Review: https://reviews.apache.org/r/11108
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 
> 
> Diff: https://reviews.apache.org/r/11108/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>