You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2012/10/05 03:49:08 UTC
Review Request: Fixing http download
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Ben Mahler.
Description
-------
See summary
Diffs
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12231
-----------------------------------------------------------
Ship it!
Ship It!
- Florian Leibert
On Oct. 8, 2012, 5:33 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler.
>
>
> Description
> -------
>
> See summary
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote:
> > None are yours but let's clean this up while we're at it!
It would also be good to flip the default in configure.ac to include curl in either this or a separate review.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12229
-----------------------------------------------------------
On Oct. 8, 2012, 5:33 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler.
>
>
> Description
> -------
>
> See summary
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Better error messages
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 35
> > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line35>
> >
> > While we're here, I would wrap this with an additional error string, two options:
> >
> > 1. Wrap the error: "Failed to open file '<path>': " + fd.error()
> > 2. Change os::open to be more descriptive and just have this return fd directly. Not sure about this option since we don't have a consistent error format in os.hpp, some are raw strerror, some are wrapped with more info.
done and done
> On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 43
> > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line43>
> >
> > I missed this in my Try refactor:
> >
> > Try<Nothing> close = os::close(fd.get())
> >
> > // Then I guess ignore the close error since we've failed at this point anyway.
why do we need to store it in a variable?
> On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 68
> > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line68>
> >
> > Seems that we should return an error when the response code is anything other than 200 OK.
> >
> > Maybe we return a Try<Nothing>? What do you think?
> >
> > Currently it is not documented what this returns, can you update the doc at the top?
I think returning the response code is valuable, the way we do it here. The user can decide what he wants to do based on the response.
This is documented at top, not sure what you want?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12229
-----------------------------------------------------------
On Oct. 8, 2012, 7:27 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:27 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12229
-----------------------------------------------------------
None are yours but let's clean this up while we're at it!
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25954>
While we're here, I would wrap this with an additional error string, two options:
1. Wrap the error: "Failed to open file '<path>': " + fd.error()
2. Change os::open to be more descriptive and just have this return fd directly. Not sure about this option since we don't have a consistent error format in os.hpp, some are raw strerror, some are wrapped with more info.
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25955>
I missed this in my Try refactor:
Try<Nothing> close = os::close(fd.get())
// Then I guess ignore the close error since we've failed at this point anyway.
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25956>
Handle null FILE* case and fail accordingly.
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25958>
Seems that we should return an error when the response code is anything other than 200 OK.
Maybe we return a Try<Nothing>? What do you think?
Currently it is not documented what this returns, can you update the doc at the top?
- Ben Mahler
On Oct. 8, 2012, 5:33 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler.
>
>
> Description
> -------
>
> See summary
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12242
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
> On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
> > I just tried to apply the patch from this review and compile failed - haven't had a chance to dig yet but it seems related:
> >
> > \" -DBUILD_DIR=\"/srv/mesos/src/build\" -I../third_party/gmock-1.6.0/gtest/include -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\" -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF tests/.deps/mesos_tests-stout_tests.Tpo -c -o tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo '/srv/mesos/src/src/'`tests/stout_tests.cpp
> > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
> > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized const 'EMPTY' [-fpermissive]
> > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note: 'const class hashset<std::basic_string<char> >' has no user-provided default constructor
> > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
> > make[3]: Leaving directory `/srv/mesos/src/build/src'
> > make[2]: *** [check-am] Error 2
> > make[2]: Leaving directory `/srv/mesos/src/build/src'
> > make[1]: *** [check] Error 2
> > make[1]: Leaving directory `/srv/mesos/src/build/src'
> > make: *** [check-recursive] Error 1
> > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return code 2
> >
Sorry for the extra output - this comes from my cloud-init scripts and is mixed into the compile output...
- Florian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12277
-----------------------------------------------------------
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
> > I just tried to apply the patch from this review and compile failed - haven't had a chance to dig yet but it seems related:
> >
> > \" -DBUILD_DIR=\"/srv/mesos/src/build\" -I../third_party/gmock-1.6.0/gtest/include -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\" -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF tests/.deps/mesos_tests-stout_tests.Tpo -c -o tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo '/srv/mesos/src/src/'`tests/stout_tests.cpp
> > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
> > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized const 'EMPTY' [-fpermissive]
> > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note: 'const class hashset<std::basic_string<char> >' has no user-provided default constructor
> > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
> > make[3]: Leaving directory `/srv/mesos/src/build/src'
> > make[2]: *** [check-am] Error 2
> > make[2]: Leaving directory `/srv/mesos/src/build/src'
> > make[1]: *** [check] Error 2
> > make[1]: Leaving directory `/srv/mesos/src/build/src'
> > make: *** [check-recursive] Error 1
> > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return code 2
> >
>
> Florian Leibert wrote:
> Sorry for the extra output - this comes from my cloud-init scripts and is mixed into the compile output...
>
> Vinod Kone wrote:
> this doesnt look related! surprised that you got this error only after applying the above patch. let me try to repro...
couldn't repro this on osx (10.6) or centos (5.5). can you let me know what's your environment and the commands that you used to build mesos?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12277
-----------------------------------------------------------
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
> > I just tried to apply the patch from this review and compile failed - haven't had a chance to dig yet but it seems related:
> >
> > \" -DBUILD_DIR=\"/srv/mesos/src/build\" -I../third_party/gmock-1.6.0/gtest/include -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\" -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF tests/.deps/mesos_tests-stout_tests.Tpo -c -o tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo '/srv/mesos/src/src/'`tests/stout_tests.cpp
> > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
> > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized const 'EMPTY' [-fpermissive]
> > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note: 'const class hashset<std::basic_string<char> >' has no user-provided default constructor
> > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
> > make[3]: Leaving directory `/srv/mesos/src/build/src'
> > make[2]: *** [check-am] Error 2
> > make[2]: Leaving directory `/srv/mesos/src/build/src'
> > make[1]: *** [check] Error 2
> > make[1]: Leaving directory `/srv/mesos/src/build/src'
> > make: *** [check-recursive] Error 1
> > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return code 2
> >
>
> Florian Leibert wrote:
> Sorry for the extra output - this comes from my cloud-init scripts and is mixed into the compile output...
this doesnt look related! surprised that you got this error only after applying the above patch. let me try to repro...
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12277
-----------------------------------------------------------
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Benjamin Mahler <bm...@twitter.com>.
Interesting, that's likely because either (1) the CL is not up to date with
trunk, or (2) 4.7 has a regression for catching this issue.
On Tue, Oct 9, 2012 at 2:16 PM, Florian Leibert <fl...@leibert.de> wrote:
> I tried getting this to work with both gcc 4.4 & 4.5 and can't make that
> happen either.
>
>
> On Tue, Oct 9, 2012 at 2:12 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>
>> We do have a CL for support with 4.7: https://reviews.apache.org/r/6988
>>
>> But we don't currently compile our changes with 4.7 during development. I
>> think we're all ready and willing to make the switch soon. We don't have
>> 4.7 on our CentOS installs currently so we'll have to deal with that first
>> I imagine.
>>
>> On Tue, Oct 9, 2012 at 1:47 PM, Florian Leibert <fl...@leibert.de> wrote:
>>
>>> That's a bummer - none of the current Ubuntu versions ship with 4.1. Do
>>> you have a list of versions you guys support?
>>>
>>>
>>> On Tue, Oct 9, 2012 at 12:42 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>>
>>>> Until we make the switch to gcc 4.7 you'll likely continue to run into
>>>> some issues building off trunk. We use gcc 4.1.2 on centos currently.
>>>>
>>>>
>>>> On Tue, Oct 9, 2012 at 12:24 PM, Florian Leibert <fl...@leibert.de>wrote:
>>>>
>>>>> root@i-f090d68d:/srv/mesos/src/build# g++ --version
>>>>> g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
>>>>> Copyright (C) 2011 Free Software Foundation, Inc.
>>>>> This is free software; see the source for copying conditions. There
>>>>> is NO
>>>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>>>>> PURPOSE.
>>>>>
>>>>>
>>>>> On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>>>>
>>>>>> Vinod is working on it now.
>>>>>>
>>>>>> This is definitely in the standard:
>>>>>> "If a program calls for the default initialization of an object of a
>>>>>> const-qualified type T, T shall be a class type with a user-provided
>>>>>> default constructor."
>>>>>>
>>>>>> I'm curious what version of gcc are you using?
>>>>>>
>>>>>> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de>wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>>>>>>> > > I just tried to apply the patch from this review and compile
>>>>>>> failed - haven't had a chance to dig yet but it seems related:
>>>>>>> > >
>>>>>>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>>>>>>> -I../third_party/gmock-1.6.0/gtest/include
>>>>>>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>>>>>>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>>>>>>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>>>>>>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>>>>>>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>>>>>>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>>>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function
>>>>>>> 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
>>>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error:
>>>>>>> uninitialized const 'EMPTY' [-fpermissive]
>>>>>>> > >
>>>>>>> /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note:
>>>>>>> 'const class hashset<std::basic_string<char> >' has no user-provided
>>>>>>> default constructor
>>>>>>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>>>>>>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>>>>>>> > > make[2]: *** [check-am] Error 2
>>>>>>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>>>>>>> > > make[1]: *** [check] Error 2
>>>>>>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>>>>>>> > > make: *** [check-recursive] Error 1
>>>>>>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>>>>>>> return code 2
>>>>>>> > >
>>>>>>> >
>>>>>>> > Florian Leibert wrote:
>>>>>>> > Sorry for the extra output - this comes from my cloud-init
>>>>>>> scripts and is mixed into the compile output...
>>>>>>> >
>>>>>>> > Vinod Kone wrote:
>>>>>>> > this doesnt look related! surprised that you got this error
>>>>>>> only after applying the above patch. let me try to repro...
>>>>>>> >
>>>>>>> > Vinod Kone wrote:
>>>>>>> > couldn't repro this on osx (10.6) or centos (5.5). can you let
>>>>>>> me know what's your environment and the commands that you used to build
>>>>>>> mesos?
>>>>>>>
>>>>>>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted
>>>>>>> the patch and still get the error. I must have been on a different git
>>>>>>> revision last week...
>>>>>>> But either way - it would be great if you guys could look into this
>>>>>>> bug I filed as it's a blocker for us.
>>>>>>>
>>>>>>>
>>>>>>> - Florian
>>>>>>>
>>>>>>>
>>>>>>> -----------------------------------------------------------
>>>>>>>
>>>>>>> This is an automatically generated e-mail. To reply, visit:
>>>>>>> https://reviews.apache.org/r/7454/#review12277
>>>>>>> -----------------------------------------------------------
>>>>>>>
>>>>>>>
>>>>>>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>>>>>>> >
>>>>>>> > -----------------------------------------------------------
>>>>>>>
>>>>>>> > This is an automatically generated e-mail. To reply, visit:
>>>>>>> > https://reviews.apache.org/r/7454/
>>>>>>> > -----------------------------------------------------------
>>>>>>> >
>>>>>>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>>>>>>
>>>>>>> >
>>>>>>> >
>>>>>>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>>>>>>> >
>>>>>>> >
>>>>>>> > Description
>>>>>>> > -------
>>>>>>>
>>>>>>> >
>>>>>>> > Better error messages
>>>>>>> >
>>>>>>> >
>>>>>>> > Fix for curl download
>>>>>>> >
>>>>>>> >
>>>>>>> > Diffs
>>>>>>> > -----
>>>>>>> >
>>>>>>> > src/launcher/launcher.cpp
>>>>>>> 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>>>>>>> > third_party/libprocess/include/stout/net.hpp
>>>>>>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>>>>>>> > third_party/libprocess/include/stout/os.hpp
>>>>>>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>>>>>>> >
>>>>>>> > Diff: https://reviews.apache.org/r/7454/diff/
>>>>>>> >
>>>>>>> >
>>>>>>> > Testing
>>>>>>> > -------
>>>>>>> >
>>>>>>> > Tested locally
>>>>>>> >
>>>>>>> >
>>>>>>> > Thanks,
>>>>>>> >
>>>>>>> > Vinod Kone
>>>>>>> >
>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards,
>>>>>
>>>>> Florian
>>>>> http://twitter.com/flo <http://twitter.com/floleibert>
>>>>> http://flori.posterous.com/
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>>
>>> Florian
>>> http://twitter.com/flo <http://twitter.com/floleibert>
>>> http://flori.posterous.com/
>>>
>>>
>>
>
>
> --
> Best regards,
>
> Florian
> http://twitter.com/flo <http://twitter.com/floleibert>
> http://flori.posterous.com/
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
I tried getting this to work with both gcc 4.4 & 4.5 and can't make that
happen either.
On Tue, Oct 9, 2012 at 2:12 PM, Benjamin Mahler <bm...@twitter.com> wrote:
> We do have a CL for support with 4.7: https://reviews.apache.org/r/6988
>
> But we don't currently compile our changes with 4.7 during development. I
> think we're all ready and willing to make the switch soon. We don't have
> 4.7 on our CentOS installs currently so we'll have to deal with that first
> I imagine.
>
> On Tue, Oct 9, 2012 at 1:47 PM, Florian Leibert <fl...@leibert.de> wrote:
>
>> That's a bummer - none of the current Ubuntu versions ship with 4.1. Do
>> you have a list of versions you guys support?
>>
>>
>> On Tue, Oct 9, 2012 at 12:42 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>
>>> Until we make the switch to gcc 4.7 you'll likely continue to run into
>>> some issues building off trunk. We use gcc 4.1.2 on centos currently.
>>>
>>>
>>> On Tue, Oct 9, 2012 at 12:24 PM, Florian Leibert <fl...@leibert.de> wrote:
>>>
>>>> root@i-f090d68d:/srv/mesos/src/build# g++ --version
>>>> g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
>>>> Copyright (C) 2011 Free Software Foundation, Inc.
>>>> This is free software; see the source for copying conditions. There is
>>>> NO
>>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>>>> PURPOSE.
>>>>
>>>>
>>>> On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>>>
>>>>> Vinod is working on it now.
>>>>>
>>>>> This is definitely in the standard:
>>>>> "If a program calls for the default initialization of an object of a
>>>>> const-qualified type T, T shall be a class type with a user-provided
>>>>> default constructor."
>>>>>
>>>>> I'm curious what version of gcc are you using?
>>>>>
>>>>> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de>wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>>>>>> > > I just tried to apply the patch from this review and compile
>>>>>> failed - haven't had a chance to dig yet but it seems related:
>>>>>> > >
>>>>>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>>>>>> -I../third_party/gmock-1.6.0/gtest/include
>>>>>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>>>>>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>>>>>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>>>>>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>>>>>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>>>>>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function
>>>>>> 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
>>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error:
>>>>>> uninitialized const 'EMPTY' [-fpermissive]
>>>>>> > >
>>>>>> /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note:
>>>>>> 'const class hashset<std::basic_string<char> >' has no user-provided
>>>>>> default constructor
>>>>>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>>>>>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>>>>>> > > make[2]: *** [check-am] Error 2
>>>>>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>>>>>> > > make[1]: *** [check] Error 2
>>>>>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>>>>>> > > make: *** [check-recursive] Error 1
>>>>>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>>>>>> return code 2
>>>>>> > >
>>>>>> >
>>>>>> > Florian Leibert wrote:
>>>>>> > Sorry for the extra output - this comes from my cloud-init
>>>>>> scripts and is mixed into the compile output...
>>>>>> >
>>>>>> > Vinod Kone wrote:
>>>>>> > this doesnt look related! surprised that you got this error
>>>>>> only after applying the above patch. let me try to repro...
>>>>>> >
>>>>>> > Vinod Kone wrote:
>>>>>> > couldn't repro this on osx (10.6) or centos (5.5). can you let
>>>>>> me know what's your environment and the commands that you used to build
>>>>>> mesos?
>>>>>>
>>>>>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
>>>>>> patch and still get the error. I must have been on a different git revision
>>>>>> last week...
>>>>>> But either way - it would be great if you guys could look into this
>>>>>> bug I filed as it's a blocker for us.
>>>>>>
>>>>>>
>>>>>> - Florian
>>>>>>
>>>>>>
>>>>>> -----------------------------------------------------------
>>>>>>
>>>>>> This is an automatically generated e-mail. To reply, visit:
>>>>>> https://reviews.apache.org/r/7454/#review12277
>>>>>> -----------------------------------------------------------
>>>>>>
>>>>>>
>>>>>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>>>>>> >
>>>>>> > -----------------------------------------------------------
>>>>>>
>>>>>> > This is an automatically generated e-mail. To reply, visit:
>>>>>> > https://reviews.apache.org/r/7454/
>>>>>> > -----------------------------------------------------------
>>>>>> >
>>>>>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>>>>>
>>>>>> >
>>>>>> >
>>>>>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>>>>>> >
>>>>>> >
>>>>>> > Description
>>>>>> > -------
>>>>>>
>>>>>> >
>>>>>> > Better error messages
>>>>>> >
>>>>>> >
>>>>>> > Fix for curl download
>>>>>> >
>>>>>> >
>>>>>> > Diffs
>>>>>> > -----
>>>>>> >
>>>>>> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>>>>>> > third_party/libprocess/include/stout/net.hpp
>>>>>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>>>>>> > third_party/libprocess/include/stout/os.hpp
>>>>>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>>>>>> >
>>>>>> > Diff: https://reviews.apache.org/r/7454/diff/
>>>>>> >
>>>>>> >
>>>>>> > Testing
>>>>>> > -------
>>>>>> >
>>>>>> > Tested locally
>>>>>> >
>>>>>> >
>>>>>> > Thanks,
>>>>>> >
>>>>>> > Vinod Kone
>>>>>> >
>>>>>> >
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>>
>>>> Florian
>>>> http://twitter.com/flo <http://twitter.com/floleibert>
>>>> http://flori.posterous.com/
>>>>
>>>>
>>>
>>
>>
>> --
>> Best regards,
>>
>> Florian
>> http://twitter.com/flo <http://twitter.com/floleibert>
>> http://flori.posterous.com/
>>
>>
>
--
Best regards,
Florian
http://twitter.com/flo <http://twitter.com/floleibert>
http://flori.posterous.com/
Re: Review Request: Fixing http download
Posted by Benjamin Mahler <bm...@twitter.com>.
We do have a CL for support with 4.7: https://reviews.apache.org/r/6988
But we don't currently compile our changes with 4.7 during development. I
think we're all ready and willing to make the switch soon. We don't have
4.7 on our CentOS installs currently so we'll have to deal with that first
I imagine.
On Tue, Oct 9, 2012 at 1:47 PM, Florian Leibert <fl...@leibert.de> wrote:
> That's a bummer - none of the current Ubuntu versions ship with 4.1. Do
> you have a list of versions you guys support?
>
>
> On Tue, Oct 9, 2012 at 12:42 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>
>> Until we make the switch to gcc 4.7 you'll likely continue to run into
>> some issues building off trunk. We use gcc 4.1.2 on centos currently.
>>
>>
>> On Tue, Oct 9, 2012 at 12:24 PM, Florian Leibert <fl...@leibert.de> wrote:
>>
>>> root@i-f090d68d:/srv/mesos/src/build# g++ --version
>>> g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
>>> Copyright (C) 2011 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions. There is
>>> NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>>> PURPOSE.
>>>
>>>
>>> On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>>
>>>> Vinod is working on it now.
>>>>
>>>> This is definitely in the standard:
>>>> "If a program calls for the default initialization of an object of a
>>>> const-qualified type T, T shall be a class type with a user-provided
>>>> default constructor."
>>>>
>>>> I'm curious what version of gcc are you using?
>>>>
>>>> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de>wrote:
>>>>
>>>>>
>>>>>
>>>>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>>>>> > > I just tried to apply the patch from this review and compile
>>>>> failed - haven't had a chance to dig yet but it seems related:
>>>>> > >
>>>>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>>>>> -I../third_party/gmock-1.6.0/gtest/include
>>>>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>>>>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>>>>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>>>>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>>>>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>>>>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function
>>>>> 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
>>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error:
>>>>> uninitialized const 'EMPTY' [-fpermissive]
>>>>> > >
>>>>> /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note:
>>>>> 'const class hashset<std::basic_string<char> >' has no user-provided
>>>>> default constructor
>>>>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>>>>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>>>>> > > make[2]: *** [check-am] Error 2
>>>>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>>>>> > > make[1]: *** [check] Error 2
>>>>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>>>>> > > make: *** [check-recursive] Error 1
>>>>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>>>>> return code 2
>>>>> > >
>>>>> >
>>>>> > Florian Leibert wrote:
>>>>> > Sorry for the extra output - this comes from my cloud-init
>>>>> scripts and is mixed into the compile output...
>>>>> >
>>>>> > Vinod Kone wrote:
>>>>> > this doesnt look related! surprised that you got this error only
>>>>> after applying the above patch. let me try to repro...
>>>>> >
>>>>> > Vinod Kone wrote:
>>>>> > couldn't repro this on osx (10.6) or centos (5.5). can you let
>>>>> me know what's your environment and the commands that you used to build
>>>>> mesos?
>>>>>
>>>>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
>>>>> patch and still get the error. I must have been on a different git revision
>>>>> last week...
>>>>> But either way - it would be great if you guys could look into this
>>>>> bug I filed as it's a blocker for us.
>>>>>
>>>>>
>>>>> - Florian
>>>>>
>>>>>
>>>>> -----------------------------------------------------------
>>>>>
>>>>> This is an automatically generated e-mail. To reply, visit:
>>>>> https://reviews.apache.org/r/7454/#review12277
>>>>> -----------------------------------------------------------
>>>>>
>>>>>
>>>>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>>>>> >
>>>>> > -----------------------------------------------------------
>>>>>
>>>>> > This is an automatically generated e-mail. To reply, visit:
>>>>> > https://reviews.apache.org/r/7454/
>>>>> > -----------------------------------------------------------
>>>>> >
>>>>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>>>>
>>>>> >
>>>>> >
>>>>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>>>>> >
>>>>> >
>>>>> > Description
>>>>> > -------
>>>>>
>>>>> >
>>>>> > Better error messages
>>>>> >
>>>>> >
>>>>> > Fix for curl download
>>>>> >
>>>>> >
>>>>> > Diffs
>>>>> > -----
>>>>> >
>>>>> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>>>>> > third_party/libprocess/include/stout/net.hpp
>>>>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>>>>> > third_party/libprocess/include/stout/os.hpp
>>>>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>>>>> >
>>>>> > Diff: https://reviews.apache.org/r/7454/diff/
>>>>> >
>>>>> >
>>>>> > Testing
>>>>> > -------
>>>>> >
>>>>> > Tested locally
>>>>> >
>>>>> >
>>>>> > Thanks,
>>>>> >
>>>>> > Vinod Kone
>>>>> >
>>>>> >
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>>
>>> Florian
>>> http://twitter.com/flo <http://twitter.com/floleibert>
>>> http://flori.posterous.com/
>>>
>>>
>>
>
>
> --
> Best regards,
>
> Florian
> http://twitter.com/flo <http://twitter.com/floleibert>
> http://flori.posterous.com/
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
That's a bummer - none of the current Ubuntu versions ship with 4.1. Do you
have a list of versions you guys support?
On Tue, Oct 9, 2012 at 12:42 PM, Benjamin Mahler <bm...@twitter.com>wrote:
> Until we make the switch to gcc 4.7 you'll likely continue to run into
> some issues building off trunk. We use gcc 4.1.2 on centos currently.
>
>
> On Tue, Oct 9, 2012 at 12:24 PM, Florian Leibert <fl...@leibert.de> wrote:
>
>> root@i-f090d68d:/srv/mesos/src/build# g++ --version
>> g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
>> Copyright (C) 2011 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions. There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>> PURPOSE.
>>
>>
>> On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>>
>>> Vinod is working on it now.
>>>
>>> This is definitely in the standard:
>>> "If a program calls for the default initialization of an object of a
>>> const-qualified type T, T shall be a class type with a user-provided
>>> default constructor."
>>>
>>> I'm curious what version of gcc are you using?
>>>
>>> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de> wrote:
>>>
>>>>
>>>>
>>>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>>>> > > I just tried to apply the patch from this review and compile failed
>>>> - haven't had a chance to dig yet but it seems related:
>>>> > >
>>>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>>>> -I../third_party/gmock-1.6.0/gtest/include
>>>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>>>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>>>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>>>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>>>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>>>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function
>>>> 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
>>>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error:
>>>> uninitialized const 'EMPTY' [-fpermissive]
>>>> > >
>>>> /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note:
>>>> 'const class hashset<std::basic_string<char> >' has no user-provided
>>>> default constructor
>>>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>>>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>>>> > > make[2]: *** [check-am] Error 2
>>>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>>>> > > make[1]: *** [check] Error 2
>>>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>>>> > > make: *** [check-recursive] Error 1
>>>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>>>> return code 2
>>>> > >
>>>> >
>>>> > Florian Leibert wrote:
>>>> > Sorry for the extra output - this comes from my cloud-init
>>>> scripts and is mixed into the compile output...
>>>> >
>>>> > Vinod Kone wrote:
>>>> > this doesnt look related! surprised that you got this error only
>>>> after applying the above patch. let me try to repro...
>>>> >
>>>> > Vinod Kone wrote:
>>>> > couldn't repro this on osx (10.6) or centos (5.5). can you let me
>>>> know what's your environment and the commands that you used to build mesos?
>>>>
>>>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
>>>> patch and still get the error. I must have been on a different git revision
>>>> last week...
>>>> But either way - it would be great if you guys could look into this bug
>>>> I filed as it's a blocker for us.
>>>>
>>>>
>>>> - Florian
>>>>
>>>>
>>>> -----------------------------------------------------------
>>>>
>>>> This is an automatically generated e-mail. To reply, visit:
>>>> https://reviews.apache.org/r/7454/#review12277
>>>> -----------------------------------------------------------
>>>>
>>>>
>>>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>>>> >
>>>> > -----------------------------------------------------------
>>>>
>>>> > This is an automatically generated e-mail. To reply, visit:
>>>> > https://reviews.apache.org/r/7454/
>>>> > -----------------------------------------------------------
>>>> >
>>>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>>>
>>>> >
>>>> >
>>>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>>>> >
>>>> >
>>>> > Description
>>>> > -------
>>>>
>>>> >
>>>> > Better error messages
>>>> >
>>>> >
>>>> > Fix for curl download
>>>> >
>>>> >
>>>> > Diffs
>>>> > -----
>>>> >
>>>> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>>>> > third_party/libprocess/include/stout/net.hpp
>>>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>>>> > third_party/libprocess/include/stout/os.hpp
>>>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>>>> >
>>>> > Diff: https://reviews.apache.org/r/7454/diff/
>>>> >
>>>> >
>>>> > Testing
>>>> > -------
>>>> >
>>>> > Tested locally
>>>> >
>>>> >
>>>> > Thanks,
>>>> >
>>>> > Vinod Kone
>>>> >
>>>> >
>>>>
>>>>
>>>
>>
>>
>> --
>> Best regards,
>>
>> Florian
>> http://twitter.com/flo <http://twitter.com/floleibert>
>> http://flori.posterous.com/
>>
>>
>
--
Best regards,
Florian
http://twitter.com/flo <http://twitter.com/floleibert>
http://flori.posterous.com/
Re: Review Request: Fixing http download
Posted by Benjamin Mahler <bm...@twitter.com>.
Until we make the switch to gcc 4.7 you'll likely continue to run into some
issues building off trunk. We use gcc 4.1.2 on centos currently.
On Tue, Oct 9, 2012 at 12:24 PM, Florian Leibert <fl...@leibert.de> wrote:
> root@i-f090d68d:/srv/mesos/src/build# g++ --version
> g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
> Copyright (C) 2011 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
>
> On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
>
>> Vinod is working on it now.
>>
>> This is definitely in the standard:
>> "If a program calls for the default initialization of an object of a
>> const-qualified type T, T shall be a class type with a user-provided
>> default constructor."
>>
>> I'm curious what version of gcc are you using?
>>
>> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de> wrote:
>>
>>>
>>>
>>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>>> > > I just tried to apply the patch from this review and compile failed
>>> - haven't had a chance to dig yet but it seems related:
>>> > >
>>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>>> -I../third_party/gmock-1.6.0/gtest/include
>>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function
>>> 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
>>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error:
>>> uninitialized const 'EMPTY' [-fpermissive]
>>> > >
>>> /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note:
>>> 'const class hashset<std::basic_string<char> >' has no user-provided
>>> default constructor
>>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>>> > > make[2]: *** [check-am] Error 2
>>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>>> > > make[1]: *** [check] Error 2
>>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>>> > > make: *** [check-recursive] Error 1
>>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>>> return code 2
>>> > >
>>> >
>>> > Florian Leibert wrote:
>>> > Sorry for the extra output - this comes from my cloud-init scripts
>>> and is mixed into the compile output...
>>> >
>>> > Vinod Kone wrote:
>>> > this doesnt look related! surprised that you got this error only
>>> after applying the above patch. let me try to repro...
>>> >
>>> > Vinod Kone wrote:
>>> > couldn't repro this on osx (10.6) or centos (5.5). can you let me
>>> know what's your environment and the commands that you used to build mesos?
>>>
>>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
>>> patch and still get the error. I must have been on a different git revision
>>> last week...
>>> But either way - it would be great if you guys could look into this bug
>>> I filed as it's a blocker for us.
>>>
>>>
>>> - Florian
>>>
>>>
>>> -----------------------------------------------------------
>>>
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/7454/#review12277
>>> -----------------------------------------------------------
>>>
>>>
>>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>>> >
>>> > -----------------------------------------------------------
>>>
>>> > This is an automatically generated e-mail. To reply, visit:
>>> > https://reviews.apache.org/r/7454/
>>> > -----------------------------------------------------------
>>> >
>>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>>
>>> >
>>> >
>>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>>> >
>>> >
>>> > Description
>>> > -------
>>>
>>> >
>>> > Better error messages
>>> >
>>> >
>>> > Fix for curl download
>>> >
>>> >
>>> > Diffs
>>> > -----
>>> >
>>> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>>> > third_party/libprocess/include/stout/net.hpp
>>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>>> > third_party/libprocess/include/stout/os.hpp
>>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>>> >
>>> > Diff: https://reviews.apache.org/r/7454/diff/
>>> >
>>> >
>>> > Testing
>>> > -------
>>> >
>>> > Tested locally
>>> >
>>> >
>>> > Thanks,
>>> >
>>> > Vinod Kone
>>> >
>>> >
>>>
>>>
>>
>
>
> --
> Best regards,
>
> Florian
> http://twitter.com/flo <http://twitter.com/floleibert>
> http://flori.posterous.com/
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
root@i-f090d68d:/srv/mesos/src/build# g++ --version
g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
On Tue, Oct 9, 2012 at 12:14 PM, Benjamin Mahler <bm...@twitter.com>wrote:
> Vinod is working on it now.
>
> This is definitely in the standard:
> "If a program calls for the default initialization of an object of a
> const-qualified type T, T shall be a class type with a user-provided
> default constructor."
>
> I'm curious what version of gcc are you using?
>
> On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de> wrote:
>
>>
>>
>> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
>> > > I just tried to apply the patch from this review and compile failed -
>> haven't had a chance to dig yet but it seems related:
>> > >
>> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
>> -I../third_party/gmock-1.6.0/gtest/include
>> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
>> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
>> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
>> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
>> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
>> '/srv/mesos/src/src/'`tests/stout_tests.cpp
>> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual
>> void StoutUtilsTest_rmdir_Test::TestBody()':
>> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized
>> const 'EMPTY' [-fpermissive]
>> > > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7:
>> note: 'const class hashset<std::basic_string<char> >' has no user-provided
>> default constructor
>> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
>> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
>> > > make[2]: *** [check-am] Error 2
>> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
>> > > make[1]: *** [check] Error 2
>> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
>> > > make: *** [check-recursive] Error 1
>> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with
>> return code 2
>> > >
>> >
>> > Florian Leibert wrote:
>> > Sorry for the extra output - this comes from my cloud-init scripts
>> and is mixed into the compile output...
>> >
>> > Vinod Kone wrote:
>> > this doesnt look related! surprised that you got this error only
>> after applying the above patch. let me try to repro...
>> >
>> > Vinod Kone wrote:
>> > couldn't repro this on osx (10.6) or centos (5.5). can you let me
>> know what's your environment and the commands that you used to build mesos?
>>
>> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
>> patch and still get the error. I must have been on a different git revision
>> last week...
>> But either way - it would be great if you guys could look into this bug I
>> filed as it's a blocker for us.
>>
>>
>> - Florian
>>
>>
>> -----------------------------------------------------------
>>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/7454/#review12277
>> -----------------------------------------------------------
>>
>>
>> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>> >
>> > -----------------------------------------------------------
>>
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/7454/
>> > -----------------------------------------------------------
>> >
>> > (Updated Oct. 8, 2012, 7:54 p.m.)
>>
>> >
>> >
>> > Review request for mesos, Benjamin Hindman and Ben Mahler.
>> >
>> >
>> > Description
>> > -------
>>
>> >
>> > Better error messages
>> >
>> >
>> > Fix for curl download
>> >
>> >
>> > Diffs
>> > -----
>> >
>> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
>> > third_party/libprocess/include/stout/net.hpp
>> f6b770c8ca7d21e0aca0f614168a0985c77046b0
>> > third_party/libprocess/include/stout/os.hpp
>> 13dbc715ed08cfe6b24ee20744d427dac1104694
>> >
>> > Diff: https://reviews.apache.org/r/7454/diff/
>> >
>> >
>> > Testing
>> > -------
>> >
>> > Tested locally
>> >
>> >
>> > Thanks,
>> >
>> > Vinod Kone
>> >
>> >
>>
>>
>
--
Best regards,
Florian
http://twitter.com/flo <http://twitter.com/floleibert>
http://flori.posterous.com/
Re: Review Request: Fixing http download
Posted by Benjamin Mahler <bm...@twitter.com>.
Vinod is working on it now.
This is definitely in the standard:
"If a program calls for the default initialization of an object of a
const-qualified type T, T shall be a class type with a user-provided
default constructor."
I'm curious what version of gcc are you using?
On Tue, Oct 9, 2012 at 11:49 AM, Florian Leibert <fl...@leibert.de> wrote:
>
>
> > On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
> > > I just tried to apply the patch from this review and compile failed -
> haven't had a chance to dig yet but it seems related:
> > >
> > > \" -DBUILD_DIR=\"/srv/mesos/src/build\"
> -I../third_party/gmock-1.6.0/gtest/include
> -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include
> -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\"
> -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF
> tests/.deps/mesos_tests-stout_tests.Tpo -c -o
> tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo
> '/srv/mesos/src/src/'`tests/stout_tests.cpp
> > > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual
> void StoutUtilsTest_rmdir_Test::TestBody()':
> > > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized
> const 'EMPTY' [-fpermissive]
> > > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7:
> note: 'const class hashset<std::basic_string<char> >' has no user-provided
> default constructor
> > > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
> > > make[3]: Leaving directory `/srv/mesos/src/build/src'
> > > make[2]: *** [check-am] Error 2
> > > make[2]: Leaving directory `/srv/mesos/src/build/src'
> > > make[1]: *** [check] Error 2
> > > make[1]: Leaving directory `/srv/mesos/src/build/src'
> > > make: *** [check-recursive] Error 1
> > > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return
> code 2
> > >
> >
> > Florian Leibert wrote:
> > Sorry for the extra output - this comes from my cloud-init scripts
> and is mixed into the compile output...
> >
> > Vinod Kone wrote:
> > this doesnt look related! surprised that you got this error only
> after applying the above patch. let me try to repro...
> >
> > Vinod Kone wrote:
> > couldn't repro this on osx (10.6) or centos (5.5). can you let me
> know what's your environment and the commands that you used to build mesos?
>
> I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the
> patch and still get the error. I must have been on a different git revision
> last week...
> But either way - it would be great if you guys could look into this bug I
> filed as it's a blocker for us.
>
>
> - Florian
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/#review12277
> -----------------------------------------------------------
>
>
> On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/7454/
> > -----------------------------------------------------------
> >
> > (Updated Oct. 8, 2012, 7:54 p.m.)
> >
> >
> > Review request for mesos, Benjamin Hindman and Ben Mahler.
> >
> >
> > Description
> > -------
> >
> > Better error messages
> >
> >
> > Fix for curl download
> >
> >
> > Diffs
> > -----
> >
> > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> > third_party/libprocess/include/stout/net.hpp
> f6b770c8ca7d21e0aca0f614168a0985c77046b0
> > third_party/libprocess/include/stout/os.hpp
> 13dbc715ed08cfe6b24ee20744d427dac1104694
> >
> > Diff: https://reviews.apache.org/r/7454/diff/
> >
> >
> > Testing
> > -------
> >
> > Tested locally
> >
> >
> > Thanks,
> >
> > Vinod Kone
> >
> >
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
> On Oct. 9, 2012, 5:04 p.m., Florian Leibert wrote:
> > I just tried to apply the patch from this review and compile failed - haven't had a chance to dig yet but it seems related:
> >
> > \" -DBUILD_DIR=\"/srv/mesos/src/build\" -I../third_party/gmock-1.6.0/gtest/include -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\" -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF tests/.deps/mesos_tests-stout_tests.Tpo -c -o tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo '/srv/mesos/src/src/'`tests/stout_tests.cpp
> > /srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
> > /srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized const 'EMPTY' [-fpermissive]
> > /srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note: 'const class hashset<std::basic_string<char> >' has no user-provided default constructor
> > make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
> > make[3]: Leaving directory `/srv/mesos/src/build/src'
> > make[2]: *** [check-am] Error 2
> > make[2]: Leaving directory `/srv/mesos/src/build/src'
> > make[1]: *** [check] Error 2
> > make[1]: Leaving directory `/srv/mesos/src/build/src'
> > make: *** [check-recursive] Error 1
> > run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return code 2
> >
>
> Florian Leibert wrote:
> Sorry for the extra output - this comes from my cloud-init scripts and is mixed into the compile output...
>
> Vinod Kone wrote:
> this doesnt look related! surprised that you got this error only after applying the above patch. let me try to repro...
>
> Vinod Kone wrote:
> couldn't repro this on osx (10.6) or centos (5.5). can you let me know what's your environment and the commands that you used to build mesos?
I'm on mesos trunk and Ubuntu 12.04 but you're right - I reverted the patch and still get the error. I must have been on a different git revision last week...
But either way - it would be great if you guys could look into this bug I filed as it's a blocker for us.
- Florian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12277
-----------------------------------------------------------
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Florian Leibert <fl...@leibert.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12277
-----------------------------------------------------------
I just tried to apply the patch from this review and compile failed - haven't had a chance to dig yet but it seems related:
\" -DBUILD_DIR=\"/srv/mesos/src/build\" -I../third_party/gmock-1.6.0/gtest/include -I../third_party/gmock-1.6.0/include -I/usr/lib/jvm/java-7-oracle/include -I/usr/lib/jvm/java-7-oracle/include/linux -DZOOKEEPER_VERSION=\"3.3.4\" -pthread -g2 -O2 -MT tests/mesos_tests-stout_tests.o -MD -MP -MF tests/.deps/mesos_tests-stout_tests.Tpo -c -o tests/mesos_tests-stout_tests.o `test -f 'tests/stout_tests.cpp' || echo '/srv/mesos/src/src/'`tests/stout_tests.cpp
/srv/mesos/src/src/tests/stout_tests.cpp: In member function 'virtual void StoutUtilsTest_rmdir_Test::TestBody()':
/srv/mesos/src/src/tests/stout_tests.cpp:316:30: error: uninitialized const 'EMPTY' [-fpermissive]
/srv/mesos/src/third_party/libprocess/include/stout/hashset.hpp:17:7: note: 'const class hashset<std::basic_string<char> >' has no user-provided default constructor
make[3]: *** [tests/mesos_tests-stout_tests.o] Error 1
make[3]: Leaving directory `/srv/mesos/src/build/src'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/srv/mesos/src/build/src'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/srv/mesos/src/build/src'
make: *** [check-recursive] Error 1
run-parts: /var/lib/cloud/instance/scripts/part-018 exited with return code 2
- Florian Leibert
On Oct. 8, 2012, 7:54 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:54 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Fixing http download
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
(Updated Oct. 8, 2012, 7:54 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
reverting the summary back to the original.
Summary (updated)
-----------------
Fixing http download
Description
-------
Better error messages
Fix for curl download
Diffs
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone
Re: Review Request: Better error messages
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
(Updated Oct. 8, 2012, 7:53 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
moar comments
Description
-------
Better error messages
Fix for curl download
Diffs (updated)
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone
Re: Review Request: Better error messages
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 8, 2012, 7:39 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 50
> > <https://reviews.apache.org/r/7454/diff/2/?file=174758#file174758line50>
> >
> > Feel free to punt, but eventually we'd likely want fdopen, fclose wrappers in stout/fs.hpp.
i will punt for now. this review is already encompassing more than what it was intended to.
> On Oct. 8, 2012, 7:39 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 35
> > <https://reviews.apache.org/r/7454/diff/2/?file=174758#file174758line35>
> >
> > Ok now we can just return fd directly since it's a Try<int> with the appropriate error, right?
> >
> > This convention works if we've augmented all of our os Try::error strings in os, but I could be convinced otherwise :)
fixed
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12238
-----------------------------------------------------------
On Oct. 8, 2012, 7:27 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:27 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Better error messages
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/#review12238
-----------------------------------------------------------
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25974>
I guess I should have been more clear:
s/return code/HTTP Response Code
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25972>
Ok now we can just return fd directly since it's a Try<int> with the appropriate error, right?
This convention works if we've augmented all of our os Try::error strings in os, but I could be convinced otherwise :)
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/7454/#comment25973>
Feel free to punt, but eventually we'd likely want fdopen, fclose wrappers in stout/fs.hpp.
- Ben Mahler
On Oct. 8, 2012, 7:27 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7454/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 7:27 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Better error messages
>
>
> Fix for curl download
>
>
> Diffs
> -----
>
> src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
> third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
> third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
>
> Diff: https://reviews.apache.org/r/7454/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Better error messages
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
(Updated Oct. 8, 2012, 7:27 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
BenM's comments.
Improved error messages in os.hpp
Summary (updated)
-----------------
Better error messages
Description (updated)
-------
Better error messages
Fix for curl download
Diffs (updated)
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
third_party/libprocess/include/stout/os.hpp 13dbc715ed08cfe6b24ee20744d427dac1104694
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone
Re: Review Request: Fixing http download
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
(Updated Oct. 8, 2012, 5:33 p.m.)
Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler.
Changes
-------
ping
Description
-------
See summary
Diffs
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone
Re: Review Request: Fixing http download
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7454/
-----------------------------------------------------------
(Updated Oct. 5, 2012, 3:03 a.m.)
Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler.
Changes
-------
added flo to the review
Description
-------
See summary
Diffs
-----
src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee
third_party/libprocess/include/stout/net.hpp f6b770c8ca7d21e0aca0f614168a0985c77046b0
Diff: https://reviews.apache.org/r/7454/diff/
Testing
-------
Tested locally
Thanks,
Vinod Kone