You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Dan Norris <pr...@gmail.com> on 2014/05/04 02:00:52 UTC
Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/
-----------------------------------------------------------
Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
Repository: aurora
Description
-------
AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
Changes:
- Use mesos 0.18.0 instead of 0.17.0
- Download mesos source from apache.org
- Conditionally build mesos .egg
- Fix a test in test_thermos_executor.py that was broken
Diffs
-----
.mesosversion PRE-CREATION
3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
build-support/jenkins/build_executor.sh PRE-CREATION
build-support/jenkins/build_mesos.sh PRE-CREATION
build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
Diff: https://reviews.apache.org/r/21055/diff/
Testing
-------
Thanks,
Dan Norris
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Brian Wickman <wi...@apache.org>.
> On May 6, 2014, 6:59 p.m., Brian Wickman wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, line 213
> > <https://reviews.apache.org/r/21055/diff/1/?file=574139#file574139line213>
> >
> > out of curiosity, how does this ever work? checker should always be an instance and not a class, so issubclass will fail:
> >
> > >>> f = Foo()
> > >>> issubclass(f, Foo)
> > Traceback (most recent call last):
> > File "<stdin>", line 1, in <module>
> > TypeError: issubclass() arg 1 must be a class
> >
> > the constructor of ChainedStatusChecker also ensures that all inputs are instances of the StatusChecker interface:
> >
> > class ChainedStatusChecker(StatusChecker):
> > def __init__(self, status_checkers):
> > self._status_checkers = status_checkers
> > self._status = None
> > if not all(isinstance(h_i, StatusChecker) for h_i in status_checkers):
> > raise TypeError('ChainedStatusChecker must take an iterable of StatusCheckers.')
> > super(ChainedStatusChecker, self).__init__()
> >
>
> Dan Norris wrote:
> That's a copy/paste error on my part. It should have been:
> if issubclass(type(checker), StatusChecker):
>
> The whole reason for that line is to ensure that we don't try and call .name() something that was coming back as a string in te._chained_checker._status_checkers. I can send you the test output if you're interested in taking a look.
you want isinstance(checker, StatusChecker) instead (calling type(obj) is an antipattern in python.) however, this invariant is already enforced by ChainedStatusChecker, so it superfluous. it sounds like there is a legitimate bug somewhere where somebody is calling ".name" instead of ".name()" since it was recently changed from an @abstractproperty to a default method.
- Brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42340
-----------------------------------------------------------
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Dan Norris <pr...@gmail.com>.
> On May 6, 2014, 6:59 p.m., Brian Wickman wrote:
> > src/test/python/apache/aurora/executor/test_thermos_executor.py, line 213
> > <https://reviews.apache.org/r/21055/diff/1/?file=574139#file574139line213>
> >
> > out of curiosity, how does this ever work? checker should always be an instance and not a class, so issubclass will fail:
> >
> > >>> f = Foo()
> > >>> issubclass(f, Foo)
> > Traceback (most recent call last):
> > File "<stdin>", line 1, in <module>
> > TypeError: issubclass() arg 1 must be a class
> >
> > the constructor of ChainedStatusChecker also ensures that all inputs are instances of the StatusChecker interface:
> >
> > class ChainedStatusChecker(StatusChecker):
> > def __init__(self, status_checkers):
> > self._status_checkers = status_checkers
> > self._status = None
> > if not all(isinstance(h_i, StatusChecker) for h_i in status_checkers):
> > raise TypeError('ChainedStatusChecker must take an iterable of StatusCheckers.')
> > super(ChainedStatusChecker, self).__init__()
> >
That's a copy/paste error on my part. It should have been:
if issubclass(type(checker), StatusChecker):
The whole reason for that line is to ensure that we don't try and call .name() something that was coming back as a string in te._chained_checker._status_checkers. I can send you the test output if you're interested in taking a look.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42340
-----------------------------------------------------------
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42340
-----------------------------------------------------------
src/test/python/apache/aurora/executor/test_thermos_executor.py
<https://reviews.apache.org/r/21055/#comment76116>
out of curiosity, how does this ever work? checker should always be an instance and not a class, so issubclass will fail:
>>> f = Foo()
>>> issubclass(f, Foo)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: issubclass() arg 1 must be a class
the constructor of ChainedStatusChecker also ensures that all inputs are instances of the StatusChecker interface:
class ChainedStatusChecker(StatusChecker):
def __init__(self, status_checkers):
self._status_checkers = status_checkers
self._status = None
if not all(isinstance(h_i, StatusChecker) for h_i in status_checkers):
raise TypeError('ChainedStatusChecker must take an iterable of StatusCheckers.')
super(ChainedStatusChecker, self).__init__()
- Brian Wickman
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Dan Norris <pr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42097
-----------------------------------------------------------
build-support/jenkins/build_mesos.sh
<https://reviews.apache.org/r/21055/#comment75839>
I can make this a one-liner if that seems cleaner.
- Dan Norris
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Jake Farrell <jf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42098
-----------------------------------------------------------
3rdparty/python/BUILD
<https://reviews.apache.org/r/21055/#comment75840>
can not depend on the .git dir at the tlp as in the release there is no .git dir
build-support/jenkins/build_mesos.sh
<https://reviews.apache.org/r/21055/#comment75844>
Since this is only used by build_executor jenkins script does it need to be its own file?
build-support/jenkins/build_mesos.sh
<https://reviews.apache.org/r/21055/#comment75841>
space at end of line
build-support/jenkins/build_mesos.sh
<https://reviews.apache.org/r/21055/#comment75842>
this should detect python version like all other py scripts in use
src/test/python/apache/aurora/executor/test_thermos_executor.py
<https://reviews.apache.org/r/21055/#comment75843>
Nice catch
- Jake Farrell
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Jake Farrell <jf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/#review42099
-----------------------------------------------------------
Nice work Dan, overall just some minimal comments aimed more at the review group than the actual patch iteself. Thanks for picking this up and finishing the work on it
- Jake Farrell
On May 4, 2014, 12:02 a.m., Dan Norris wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21055/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 12:02 a.m.)
>
>
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
>
> I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
>
> Changes:
> - Use mesos 0.18.0 instead of 0.17.0
> - Download mesos source from apache.org
> - Conditionally build mesos .egg
> - Fix a test in test_thermos_executor.py that was broken
>
>
> Diffs
> -----
>
> .mesosversion PRE-CREATION
> 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
> build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
> build-support/jenkins/build_executor.sh PRE-CREATION
> build-support/jenkins/build_mesos.sh PRE-CREATION
> build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
> src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
>
> Diff: https://reviews.apache.org/r/21055/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
>
>
> Thanks,
>
> Dan Norris
>
>
Re: Review Request 21055: AURORA-14: Add jenkins-compatible script to build
mesos core egg and run executor-large tests
Posted by Dan Norris <pr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21055/
-----------------------------------------------------------
(Updated May 4, 2014, 12:02 a.m.)
Review request for Aurora, Jake Farrell, Kevin Sweeney, Bill Farner, and Brian Wickman.
Repository: aurora
Description
-------
AURORA-14: Add jenkins-compatible script to build mesos core egg and run executor-large tests
I took this over from wickman at his request (old review: https://reviews.apache.org/r/19955/).
Changes:
- Use mesos 0.18.0 instead of 0.17.0
- Download mesos source from apache.org
- Conditionally build mesos .egg
- Fix a test in test_thermos_executor.py that was broken
Diffs
-----
.mesosversion PRE-CREATION
3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6
build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f
build-support/jenkins/build_executor.sh PRE-CREATION
build-support/jenkins/build_mesos.sh PRE-CREATION
build.gradle 6c758f690b87eede3ae3a7c54fabac20db543840
src/test/python/apache/aurora/executor/test_thermos_executor.py 0b143d0a2cbb25c1d35f79963478800a6b6713ec
Diff: https://reviews.apache.org/r/21055/diff/
Testing (updated)
-------
./build-support/jenkins/build_mesos.sh && ./pants src/test/python/apache/aurora/executor:executor-large -vxs (what the new build_executor.sh script does)
Thanks,
Dan Norris