Review Request 39784: Upgrade Aurora to pants 0.0.55.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/ --- Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. Bugs: AURORA-1499 https://issues.apache.org/jira/browse/AURORA-1499 Repository: aurora Description --- This also brings Aurora up to pex 1.1.0 and switches to the pants setup script; an equivalent to gradlew. Of note, the script is checked in with chmod 555, its not intended to be edited. Although pants now includes checkstyle built in, conversion to use it is left for follow-on work. Also adapt make-pycharm-virtualenv which previously relied on the local bootstrapping of pants - now hidden away by the pants setup script. .pantsversion | 1 - 3rdparty/python/requirements.txt | 2 +- BUILD | 25 - BUILD.tools| 19 -- build-support/jenkins/build.sh | 2 +- build-support/pants_requirements.txt | 30 --- build-support/python/make-pycharm-virtualenv | 11 +- build-support/python/update-pants-requirements | 34 - pants | 102 +++--- pants.ini | 63 ++- src/main/python/apache/aurora/tools/BUILD | 2 +- src/main/python/apache/thermos/observer/BUILD | 2 +- 12 files changed, 114 insertions(+), 179 deletions(-) Diffs - .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 build-support/pants_requirements.txt fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 build-support/python/make-pycharm-virtualenv d7bd41835bcd9a8fe62ea522a177bfa7830a897b build-support/python/update-pants-requirements 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 src/main/python/apache/aurora/tools/BUILD e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 src/main/python/apache/thermos/observer/BUILD d7eedabc0930711530b45ac98a1159e69d1a0c00 Diff: https://reviews.apache.org/r/39784/diff/ Testing --- Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` Also generated a pycharm project via: `./build-support/python/make-pycharm-virtualenv` Confirmed library source linking worked as did running unit tests via the IDE. Also grepped for pants commands in the repo, found `binary` and `setup-py` and confirmed these worked. Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
> On Oct. 29, 2015, 4:57 p.m., Aurora ReviewBot wrote: > > Master (bcb4774) is red with this patch. > > ./build-support/jenkins/build.sh > > > > 22:56:59 00:00 [unpack-jars] > > 22:56:59 00:00 [unpack-jars] > > 22:56:59 00:00 [deferred-sources] > > 22:56:59 00:00 [deferred-sources] > > 22:56:59 00:00 [jvm-platform-validate] > > 22:56:59 00:00 [jvm-platform-validate] > > 22:56:59 00:00 [gen] > > 22:56:59 00:00 [thrift] > > 22:56:59 00:00 [protoc] > > 22:56:59 00:00 [antlr] > > 22:56:59 00:00 [ragel] > > 22:56:59 00:00 [jaxb] > > 22:57:00 00:01 [wire] > > 22:57:00 00:01 [resolve] > > 22:57:00 00:01 [ivy] > > 22:57:00 00:01 [ivy-bootstrap] > > 22:57:02 00:03 [bootstrap-nailgun-server] > > 22:57:02 00:03 [compile] > > 22:57:02 00:03 [compile] > > 22:57:02 00:03 [jvm] > > 22:57:02 00:03 [jvm-compilers] > > 22:57:02 00:03 [zinc-pre] > > 22:57:02 00:03 [zinc-post] > > 22:57:03 00:04 [jvm-dep-check] > > 22:57:03 00:04 [resources] > > 22:57:03 00:04 [prepare] > > 22:57:03 00:04 [services] > > 22:57:03 00:04 [test] > > 22:57:03 00:04 [run_prep_command] > > 22:57:03 00:04 [test] > > 22:57:03 00:04 [pytest] > > 22:57:03 00:04 [run] > > > > 22:57:07 00:08 [chroot]INFO] Attempting to fetch thrift binary > > from: > > https://dl.bintray.com/pantsbuild/bin/build-support/bin/thrift/linux/x86_64/0.9.1/thrift > > ... > > INFO] Fetched thrift binary from: > > https://dl.bintray.com/pantsbuild/bin/build-support/bin/thrift/linux/x86_64/0.9.1/thrift > > . > > > > 22:57:15 00:16 [complete][31m > >FAILURE[0m > > Exception message: Ambiguous resolvable: thrift > > > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" > > John Sirois wrote: > I need to dig in here, this is unexpected. This was pointed to a real issue, details on https://issues.apache.org/jira/browse/AURORA-1499. The short is this RB is now blocked on pants 0.0.58 which will have a fix for this: https://github.com/pantsbuild/pants/issues/2533 Pants releases every Friday, so expect updates here Friday evening. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review104495 --- On Nov. 9, 2015, 2:47 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Nov. 9, 2015, 2:47 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 27a2c77047c26ab380
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/ --- (Updated Nov. 10, 2015, 1:17 p.m.) Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. Bugs: AURORA-1499 https://issues.apache.org/jira/browse/AURORA-1499 Repository: aurora Description (updated) --- See the changelog here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.57 This also brings Aurora up to pex 1.1.0 and switches to the pants setup script; an equivalent to gradlew. Of note, the script is checked in with chmod 555, its not intended to be edited. Although pants now includes checkstyle built in, conversion to use it is left for follow-on work. Also adapt make-pycharm-virtualenv which previously relied on the local bootstrapping of pants - now hidden away by the pants setup script. .pantsversion | 1 - 3rdparty/python/requirements.txt | 2 +- BUILD | 25 - BUILD.tools| 19 -- build-support/jenkins/build.sh | 2 +- build-support/pants_requirements.txt | 30 --- build-support/python/make-pycharm-virtualenv | 11 +- build-support/python/update-pants-requirements | 34 - pants | 102 +++--- pants.ini | 63 ++- src/main/python/apache/aurora/tools/BUILD | 2 +- src/main/python/apache/thermos/observer/BUILD | 2 +- 12 files changed, 114 insertions(+), 179 deletions(-) Diffs - .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 api/src/main/thrift/org/apache/aurora/gen/BUILD 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 api/src/main/thrift/org/apache/thermos/BUILD 6b13949aceb830b962a7098ae65462151119d752 build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 build-support/pants_requirements.txt 6eefa7175f7bda7bea23466936e451113a2d86f7 build-support/python/make-pycharm-virtualenv d7bd41835bcd9a8fe62ea522a177bfa7830a897b build-support/python/update-pants-requirements 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 src/main/python/apache/aurora/tools/BUILD e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 src/main/python/apache/thermos/observer/BUILD d7eedabc0930711530b45ac98a1159e69d1a0c00 Diff: https://reviews.apache.org/r/39784/diff/ Testing --- Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` Also generated a pycharm project via: `./build-support/python/make-pycharm-virtualenv` Confirmed library source linking worked as did running unit tests via the IDE. Also grepped for pants commands in the repo, found `binary` and `setup-py` and confirmed these worked. Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/ --- (Updated Nov. 10, 2015, 1:06 p.m.) Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. Changes --- Fixup aurora api thrift deps to work with commons. Previously the aurora api specified no thrift requirement and let pants resolve it. Upgrading pants leads to a conflict with twitter.commons pinned `thrift==0.9.1` requirements so we work around by surfacing the same pinning. The resolution solution for aurora itself and any higher level clients built on aurora libs is either unchanged or of no import: + If the 3rdparty has no thrift dep they don't care by definition, the new solution will pick exactly `0.9.1` instead of floating up to - right now `0.9.3` for example - a good thing. + If the 3rdparty pins their thrift requirement, nothing changes for them whether they use `pip` or `pex` or `pants`; their higher level dep trumps since all 3 tools do a breadth 1st search and 1st level wins. Also restore a TODO to kill BUILD.tools and un-pin setuptools, let the pants defaults (more modern setuptools) work. BUILD.tools | 2 ++ api/src/main/thrift/org/apache/aurora/gen/BUILD | 7 +++ api/src/main/thrift/org/apache/thermos/BUILD| 5 - pants.ini | 7 ++- 4 files changed, 15 insertions(+), 6 deletions(-) Bugs: AURORA-1499 https://issues.apache.org/jira/browse/AURORA-1499 Repository: aurora Description --- This also brings Aurora up to pex 1.1.0 and switches to the pants setup script; an equivalent to gradlew. Of note, the script is checked in with chmod 555, its not intended to be edited. Although pants now includes checkstyle built in, conversion to use it is left for follow-on work. Also adapt make-pycharm-virtualenv which previously relied on the local bootstrapping of pants - now hidden away by the pants setup script. .pantsversion | 1 - 3rdparty/python/requirements.txt | 2 +- BUILD | 25 - BUILD.tools| 19 -- build-support/jenkins/build.sh | 2 +- build-support/pants_requirements.txt | 30 --- build-support/python/make-pycharm-virtualenv | 11 +- build-support/python/update-pants-requirements | 34 - pants | 102 +++--- pants.ini | 63 ++- src/main/python/apache/aurora/tools/BUILD | 2 +- src/main/python/apache/thermos/observer/BUILD | 2 +- 12 files changed, 114 insertions(+), 179 deletions(-) Diffs (updated) - .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 api/src/main/thrift/org/apache/aurora/gen/BUILD 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 api/src/main/thrift/org/apache/thermos/BUILD 6b13949aceb830b962a7098ae65462151119d752 build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 build-support/pants_requirements.txt 6eefa7175f7bda7bea23466936e451113a2d86f7 build-support/python/make-pycharm-virtualenv d7bd41835bcd9a8fe62ea522a177bfa7830a897b build-support/python/update-pants-requirements 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 src/main/python/apache/aurora/tools/BUILD e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 src/main/python/apache/thermos/observer/BUILD d7eedabc0930711530b45ac98a1159e69d1a0c00 Diff: https://reviews.apache.org/r/39784/diff/ Testing --- Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` Also generated a pycharm project via: `./build-support/python/make-pycharm-virtualenv` Confirmed library source linking worked as did running unit tests via the IDE. Also grepped for pants commands in the repo, found `binary` and `setup-py` and confirmed these worked. Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.57.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/ --- (Updated Nov. 10, 2015, 2:50 p.m.) Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. Changes --- Fall back to old-style resources for now. src/main/python/apache/aurora/client/cli/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Bugs: AURORA-1499 https://issues.apache.org/jira/browse/AURORA-1499 Repository: aurora Description --- See the changelog here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.57 This also brings Aurora up to pex 1.1.0 and switches to the pants setup script; an equivalent to gradlew. Of note, the script is checked in with chmod 555, its not intended to be edited. Although pants now includes checkstyle built in, conversion to use it is left for follow-on work. Also adapt make-pycharm-virtualenv which previously relied on the local bootstrapping of pants - now hidden away by the pants setup script. .pantsversion | 1 - 3rdparty/python/requirements.txt | 2 +- BUILD | 25 - BUILD.tools| 19 -- build-support/jenkins/build.sh | 2 +- build-support/pants_requirements.txt | 30 --- build-support/python/make-pycharm-virtualenv | 11 +- build-support/python/update-pants-requirements | 34 - pants | 102 +++--- pants.ini | 63 ++- src/main/python/apache/aurora/tools/BUILD | 2 +- src/main/python/apache/thermos/observer/BUILD | 2 +- 12 files changed, 114 insertions(+), 179 deletions(-) Diffs (updated) - .auroraversion d2cbead89e9c31b9fb31db9a645afb99ff585b10 .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 api/src/main/thrift/org/apache/aurora/gen/BUILD 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 api/src/main/thrift/org/apache/thermos/BUILD 6b13949aceb830b962a7098ae65462151119d752 build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 build-support/pants_requirements.txt 6eefa7175f7bda7bea23466936e451113a2d86f7 build-support/python/make-pycharm-virtualenv d7bd41835bcd9a8fe62ea522a177bfa7830a897b build-support/python/update-pants-requirements 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 src/main/python/apache/aurora/client/BUILD 8424237eb56abe38c4d0871557dc929250de1ba4 src/main/python/apache/aurora/tools/BUILD e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 src/main/python/apache/thermos/observer/BUILD d7eedabc0930711530b45ac98a1159e69d1a0c00 src/main/resources/apache/aurora/client/cli/.auroraversion src/main/resources/apache/aurora/client/cli/BUILD 7c77d10823753af56db01a3b0db49868451d6435 Diff: https://reviews.apache.org/r/39784/diff/ Testing --- Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` Also generated a pycharm project via: `./build-support/python/make-pycharm-virtualenv` Confirmed library source linking worked as did running unit tests via the IDE. Also grepped for pants commands in the repo, found `binary` and `setup-py` and confirmed these worked. Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.57.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/ --- (Updated Nov. 10, 2015, 3:58 p.m.) Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. Changes --- Fixup `.auroraversion` location in release scripts. Make the root `.auroraversion` the canonical home to simplify the release scripts and isolate the knowledge of the secondary path to the aurora cli. .auroraversion | 2 +- build-support/release/release | 3 +-- build-support/release/release-candidate | 5 ++--- src/main/python/apache/aurora/client/cli/.auroraversion | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) Bugs: AURORA-1499 https://issues.apache.org/jira/browse/AURORA-1499 Repository: aurora Description --- See the changelog here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.57 This also brings Aurora up to pex 1.1.0 and switches to the pants setup script; an equivalent to gradlew. Of note, the script is checked in with chmod 555, its not intended to be edited. Although pants now includes checkstyle built in, conversion to use it is left for follow-on work. Also adapt make-pycharm-virtualenv which previously relied on the local bootstrapping of pants - now hidden away by the pants setup script. .pantsversion | 1 - 3rdparty/python/requirements.txt | 2 +- BUILD | 25 - BUILD.tools| 19 -- build-support/jenkins/build.sh | 2 +- build-support/pants_requirements.txt | 30 --- build-support/python/make-pycharm-virtualenv | 11 +- build-support/python/update-pants-requirements | 34 - pants | 102 +++--- pants.ini | 63 ++- src/main/python/apache/aurora/tools/BUILD | 2 +- src/main/python/apache/thermos/observer/BUILD | 2 +- 12 files changed, 114 insertions(+), 179 deletions(-) Diffs (updated) - .auroraversion d2cbead89e9c31b9fb31db9a645afb99ff585b10 .auroraversion PRE-CREATION .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 api/src/main/thrift/org/apache/aurora/gen/BUILD 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 api/src/main/thrift/org/apache/thermos/BUILD 6b13949aceb830b962a7098ae65462151119d752 build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 build-support/pants_requirements.txt 6eefa7175f7bda7bea23466936e451113a2d86f7 build-support/python/make-pycharm-virtualenv d7bd41835bcd9a8fe62ea522a177bfa7830a897b build-support/python/update-pants-requirements 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec build-support/release/release 209045767a18d263814700dfb5f9912447cc1205 build-support/release/release-candidate 1caa56bfaee67dec6c70b50923b18900f88ced75 pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 src/main/python/apache/aurora/client/BUILD 8424237eb56abe38c4d0871557dc929250de1ba4 src/main/python/apache/aurora/client/cli/.auroraversion PRE-CREATION src/main/python/apache/aurora/tools/BUILD e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 src/main/python/apache/thermos/observer/BUILD d7eedabc0930711530b45ac98a1159e69d1a0c00 src/main/resources/apache/aurora/client/cli/.auroraversion 67ff1ac747879c27d7db8a84994ffd254e830a24 src/main/resources/apache/aurora/client/cli/BUILD 7c77d10823753af56db01a3b0db49868451d6435 Diff: https://reviews.apache.org/r/39784/diff/ Testing --- Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` Also generated a pycharm project via: `./build-support/python/make-pycharm-virtualenv` Confirmed library source linking worked as did running unit tests via the IDE. Also grepped for pants commands in the repo, found `binary` and `setup-py` and confirmed these worked. Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.57.
> On Nov. 10, 2015, 3:26 p.m., Zameer Manji wrote: > > Moving the `.auroraversion` file breaks the `build-support/release/release` > > and `build-support/release/release-candidate` scripts. John, I think > > updating those scripts to point to the new location is fine, but I am > > unsure on the side effects. Bill can you comment if there be any ill side > > effects from doing that? > > > > Overall this change LGTM, but I am holding back a ship because as it stands > > it will break the release scripts we have. Those scripts use the root `.auroraversion` symlink IIUC and I updated that to point to the new location. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review105977 --- On Nov. 10, 2015, 2:50 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Nov. 10, 2015, 2:50 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > See the changelog here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.57 > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .auroraversion d2cbead89e9c31b9fb31db9a645afb99ff585b10 > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > api/src/main/thrift/org/apache/aurora/gen/BUILD > 2dcc46d9658044d96f4f1b3634f29ae1d446e0d7 > api/src/main/thrift/org/apache/thermos/BUILD > 6b13949aceb830b962a7098ae65462151119d752 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > 6eefa7175f7bda7bea23466936e451113a2d86f7 > build-support/python/make-pycharm-virtualenv > d7bd41835bcd9a8fe62ea522a177bfa7830a897b > build-support/python/update-pants-requirements > 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec > pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 > pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 > src/main/python/apache/aurora/client/BUILD > 8424237eb56abe38c4d0871557dc929250de1ba4 > src/main/python/apache/aurora/tools/BUILD > e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 > src/main/python/apache/thermos/observer/BUILD > d7eedabc0930711530b45ac98a1159e69d1a0c00 > src/main/resources/apache/aurora/client/cli/.auroraversion > src/main/resources/apache/aurora/client/cli/BUILD > 7c77d10823753af56db01a3b0db49868451d6435 > > Diff: https://reviews.apache.org/r/39784/diff/ > > > Testing > --- > > Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` > > Also generated a pycharm project via: > `./build-support/python/make-pycharm-virtualenv` > Confirmed library source linking worked as did running unit tests > via the IDE. > > Also grepped for pants commands in the repo, found `binary` and `setup-py` > and confirmed these worked. > > > Thanks, > > John Sirois > >
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review105748 --- @ReviewBot retry - John Sirois On Nov. 9, 2015, 2:47 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Nov. 9, 2015, 2:47 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 27a2c77047c26ab380fc739e7c5c532bf590c6ee > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > 6eefa7175f7bda7bea23466936e451113a2d86f7 > build-support/python/make-pycharm-virtualenv > d7bd41835bcd9a8fe62ea522a177bfa7830a897b > build-support/python/update-pants-requirements > 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec > pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 > pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 > src/main/python/apache/aurora/tools/BUILD > e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 > src/main/python/apache/thermos/observer/BUILD > d7eedabc0930711530b45ac98a1159e69d1a0c00 > > Diff: https://reviews.apache.org/r/39784/diff/ > > > Testing > --- > > Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` > > Also generated a pycharm project via: > `./build-support/python/make-pycharm-virtualenv` > Confirmed library source linking worked as did running unit tests > via the IDE. > > Also grepped for pants commands in the repo, found `binary` and `setup-py` > and confirmed these worked. > > > Thanks, > > John Sirois > >
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review105743 --- @ReviewBot retry - John Sirois On Oct. 29, 2015, 4:05 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Oct. 29, 2015, 4:05 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 > build-support/python/make-pycharm-virtualenv > d7bd41835bcd9a8fe62ea522a177bfa7830a897b > build-support/python/update-pants-requirements > 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec > pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 > pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 > src/main/python/apache/aurora/tools/BUILD > e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 > src/main/python/apache/thermos/observer/BUILD > d7eedabc0930711530b45ac98a1159e69d1a0c00 > > Diff: https://reviews.apache.org/r/39784/diff/ > > > Testing > --- > > Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` > > Also generated a pycharm project via: > `./build-support/python/make-pycharm-virtualenv` > Confirmed library source linking worked as did running unit tests > via the IDE. > > Also grepped for pants commands in the repo, found `binary` and `setup-py` > and confirmed these worked. > > > Thanks, > > John Sirois > >
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants
> > plugin is the way to go here. Historically we have been very bad at
> > upgrading pants and maintaining it, I'm afraid that if we add a custom
> > plugin here we won't be able to upgrade pants in the future at all.
>
> John Sirois wrote:
> As you see fit. I will say that the APIs used here are minimal and
> historically stable. For example, Medium similarly enables checkstyle as
> well as another, non-default-installed plugin, `python-eval`, and they have
> not had to touch their plugin since initial install at 0.0.45 in August when
> it was released. The other bit to know is pants is locking down to 1.0.0 as
> the year closes to isolate more radical changes. There will be a long-term
> 1.0.0 branch the Square will be a major user of and a maintainer of with the
> primary focus being stability, secondary bugfixes, but ~no new public
> activity as the pants community charges on master towards 2.0.
>
> Zameer Manji wrote:
> Don't give up on this approach just yet, I'm curious on what the other
> reviewers have to say. I think putting the check in the pants layer is far
> better than what we have right now. I'm just not sure if we can maintain this
> going forward. If pants 1.0.0 will maintain this API that's pretty good to
> hear.
>
> John Sirois wrote:
> I did use more ceremony here than necessary. The plugin is just the
> single register.py file and the 2 entries in pants.ini. The BUILD files
> added and the maven-style directory tree in `build-support/plugins` are not
> needed. The BUILDs are only needed to support applying style checks, and
> adding tests to the custom plugin code - those could be dropped. The
> directory structure could be as simple as `build-support/pants_lugins` with
> an `__init__.py` and `register.py` in that dir and no more.
This review is being discussed on the pantsbuild slack and more discussion is
needed, but one other idea is for pants to ship the python checkstyle as a
plugin. This way folks who want to turn on checkstlye just add this to
pants.ini:
```ini
plugins: ['pantsbuild.pants.contrib.py_checkstyle==%(pants_version)s']
```
I'll provide an update when the discussion settles. Pants releases every
Friday, so if the community agrees on the plugin approach this could be ready
by tomorrow evening in `0.0.58`.
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40219/#review106301
---
On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
>
> (Updated Nov. 12, 2015, 1:54 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> ---
>
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal. Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
>
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
>
> build-support/hooks/pre-commit
>| 3 +--
> build-support/jenkins/build.sh
>| 9 +
> build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}
>| 13 ++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/__init__.py}| 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/__init__.py} | 12
> +---
> build-support/{pyth
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote: > > Although the end result LGTM, I'm not sure if having our own custom pants > > plugin is the way to go here. Historically we have been very bad at > > upgrading pants and maintaining it, I'm afraid that if we add a custom > > plugin here we won't be able to upgrade pants in the future at all. > > John Sirois wrote: > As you see fit. I will say that the APIs used here are minimal and > historically stable. For example, Medium similarly enables checkstyle as > well as another, non-default-installed plugin, `python-eval`, and they have > not had to touch their plugin since initial install at 0.0.45 in August when > it was released. The other bit to know is pants is locking down to 1.0.0 as > the year closes to isolate more radical changes. There will be a long-term > 1.0.0 branch the Square will be a major user of and a maintainer of with the > primary focus being stability, secondary bugfixes, but ~no new public > activity as the pants community charges on master towards 2.0. > > Zameer Manji wrote: > Don't give up on this approach just yet, I'm curious on what the other > reviewers have to say. I think putting the check in the pants layer is far > better than what we have right now. I'm just not sure if we can maintain this > going forward. If pants 1.0.0 will maintain this API that's pretty good to > hear. > > John Sirois wrote: > I did use more ceremony here than necessary. The plugin is just the > single register.py file and the 2 entries in pants.ini. The BUILD files > added and the maven-style directory tree in `build-support/plugins` are not > needed. The BUILDs are only needed to support applying style checks, and > adding tests to the custom plugin code - those could be dropped. The > directory structure could be as simple as `build-support/pants_lugins` with > an `__init__.py` and `register.py` in that dir and no more. > > John Sirois wrote: > This review is being discussed on the pantsbuild slack and more > discussion is needed, but one other idea is for pants to ship the python > checkstyle as a plugin. This way folks who want to turn on checkstlye just > add this to pants.ini: > ```ini > plugins: ['pantsbuild.pants.contrib.py_checkstyle==%(pants_version)s'] > ``` > > I'll provide an update when the discussion settles. Pants releases every > Friday, so if the community agrees on the plugin approach this could be ready > by tomorrow evening in `0.0.58`. > > John Sirois wrote: > Alrighty, the pantsbuild conclusion is pants should ship a plugin for > py-checkstyle instead of embedding it in core pants, but turned off and > needing a custom plugin to enable like I did here. > Please hold off on review, I'll be updating the RB tomorrow to use the > new plugin (I'm buildmeister this week, so I'm pretty confident the new > plugin will ship tomorrow ;)) I'll actually discard this review and send up a new one when the py-checkstyle plugin is released to avoid confusion. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40219/#review106301 --- On Nov. 12, 2015, 1:54 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40219/ > --- > > (Updated Nov. 12, 2015, 1:54 a.m.) > > > Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and > Zameer Manji. > > > Bugs: AURORA-1532 > https://issues.apache.org/jira/browse/AURORA-1532 > > > Repository: aurora > > > Description > --- > > The built-in pants python checkstyle plugin is turned on by adding a > small custom plugin that installs the checks in the compile goal. Now > style checks run before compile (and thus before tests) and they benefit > from fingerprinting; ie: if you test your changes, those tests will run > style checks and when you go to commit, those checks will not be re-run > by the commit hook (although files you did not test will still need to > be checked). > > A few production files were fixes up according to style failures coming > from no space after comment opening '#', unused variables, and > mis-aligned hanging closing parens. > > build-support/hooks/pre-commit >| 3 +-- > build-support/jenkins/build.sh
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants
> > plugin is the way to go here. Historically we have been very bad at
> > upgrading pants and maintaining it, I'm afraid that if we add a custom
> > plugin here we won't be able to upgrade pants in the future at all.
>
> John Sirois wrote:
> As you see fit. I will say that the APIs used here are minimal and
> historically stable. For example, Medium similarly enables checkstyle as
> well as another, non-default-installed plugin, `python-eval`, and they have
> not had to touch their plugin since initial install at 0.0.45 in August when
> it was released. The other bit to know is pants is locking down to 1.0.0 as
> the year closes to isolate more radical changes. There will be a long-term
> 1.0.0 branch the Square will be a major user of and a maintainer of with the
> primary focus being stability, secondary bugfixes, but ~no new public
> activity as the pants community charges on master towards 2.0.
>
> Zameer Manji wrote:
> Don't give up on this approach just yet, I'm curious on what the other
> reviewers have to say. I think putting the check in the pants layer is far
> better than what we have right now. I'm just not sure if we can maintain this
> going forward. If pants 1.0.0 will maintain this API that's pretty good to
> hear.
>
> John Sirois wrote:
> I did use more ceremony here than necessary. The plugin is just the
> single register.py file and the 2 entries in pants.ini. The BUILD files
> added and the maven-style directory tree in `build-support/plugins` are not
> needed. The BUILDs are only needed to support applying style checks, and
> adding tests to the custom plugin code - those could be dropped. The
> directory structure could be as simple as `build-support/pants_lugins` with
> an `__init__.py` and `register.py` in that dir and no more.
>
> John Sirois wrote:
> This review is being discussed on the pantsbuild slack and more
> discussion is needed, but one other idea is for pants to ship the python
> checkstyle as a plugin. This way folks who want to turn on checkstlye just
> add this to pants.ini:
> ```ini
> plugins: ['pantsbuild.pants.contrib.py_checkstyle==%(pants_version)s']
> ```
>
> I'll provide an update when the discussion settles. Pants releases every
> Friday, so if the community agrees on the plugin approach this could be ready
> by tomorrow evening in `0.0.58`.
Alrighty, the pantsbuild conclusion is pants should ship a plugin for
py-checkstyle instead of embedding it in core pants, but turned off and needing
a custom plugin to enable like I did here.
Please hold off on review, I'll be updating the RB tomorrow to use the new
plugin (I'm buildmeister this week, so I'm pretty confident the new plugin will
ship tomorrow ;))
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40219/#review106301
---
On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
>
> (Updated Nov. 12, 2015, 1:54 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> ---
>
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal. Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
>
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
>
> build-support/hooks/pre-commit
>| 3 +--
> build-support/jenkins/build.sh
>| 9 +
> build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}
>
Re: Review Request 40220: Modernize the pex venv script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40220/ --- (Updated Nov. 12, 2015, 1:35 a.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Changes --- Modernize the pex venv script. This converts from grabbing the old `twitter.common.python` pex to grabbing modern pex to match the version specified in 3rdparty to help keep the pex venv up to date with the codebase dependencies. build-support/pex | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) Repository: aurora Description --- This converts from grabbing the old Twitter python pex to grabbing modern pex to match the version specified in 3rdparty to help keep the pex venv up to date with the codebase dependencies. build-support/pex | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Diffs (updated) - build-support/pex 54e31f11f152cada3809ebd7b2cdec1d2ba12ac9 Diff: https://reviews.apache.org/r/40220/diff/ Testing --- Locally ran this 2x and observed both the proper version (1.1.0) and proper use of the cached venv in the second run: `git clean -fdx build-support && ./build-support/pex --version` Thanks, John Sirois
Re: Review Request 40220: Modernize the pex venv script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40220/#review106210 --- Sorry about that - diff 2 is what you want to look at: https://reviews.apache.org/r/40220/diff/2#index_header - John Sirois On Nov. 12, 2015, 1:35 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40220/ > --- > > (Updated Nov. 12, 2015, 1:35 a.m.) > > > Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. > > > Repository: aurora > > > Description > --- > > This converts from grabbing the old Twitter python pex to grabbing > modern pex to match the version specified in 3rdparty to help keep the > pex venv up to date with the codebase dependencies. > > build-support/pex | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > > Diffs > - > > build-support/pex 54e31f11f152cada3809ebd7b2cdec1d2ba12ac9 > > Diff: https://reviews.apache.org/r/40220/diff/ > > > Testing > --- > > Locally ran this 2x and observed both the proper version (1.1.0) and > proper use of the cached venv in the second run: > `git clean -fdx build-support && ./build-support/pex --version` > > > Thanks, > > John Sirois > >
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants
> > plugin is the way to go here. Historically we have been very bad at
> > upgrading pants and maintaining it, I'm afraid that if we add a custom
> > plugin here we won't be able to upgrade pants in the future at all.
>
> John Sirois wrote:
> As you see fit. I will say that the APIs used here are minimal and
> historically stable. For example, Medium similarly enables checkstyle as
> well as another, non-default-installed plugin, `python-eval`, and they have
> not had to touch their plugin since initial install at 0.0.45 in August when
> it was released. The other bit to know is pants is locking down to 1.0.0 as
> the year closes to isolate more radical changes. There will be a long-term
> 1.0.0 branch the Square will be a major user of and a maintainer of with the
> primary focus being stability, secondary bugfixes, but ~no new public
> activity as the pants community charges on master towards 2.0.
>
> Zameer Manji wrote:
> Don't give up on this approach just yet, I'm curious on what the other
> reviewers have to say. I think putting the check in the pants layer is far
> better than what we have right now. I'm just not sure if we can maintain this
> going forward. If pants 1.0.0 will maintain this API that's pretty good to
> hear.
I did use more ceremony here than necessary. The plugin is just the single
register.py file and the 2 entries in pants.ini. The BUILD files added and the
maven-style directory tree in `build-support/plugins` are not needed. The
BUILDs are only needed to support applying style checks, and adding tests to
the custom plugin code - those could be dropped. The directory structure could
be as simple as `build-support/pants_lugins` with an `__init__.py` and
`register.py` in that dir and no more.
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40219/#review106301
-------
On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
>
> (Updated Nov. 12, 2015, 1:54 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> ---
>
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal. Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
>
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
>
> build-support/hooks/pre-commit
>| 3 +--
> build-support/jenkins/build.sh
>| 9 +
> build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}
>| 13 ++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/__init__.py}| 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/BUILD} | 18
> +++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12
> +---
>
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
> | 36
> build-support/python/checkstyle
>|
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
> On Nov. 13, 2015, 9:55 a.m., John Sirois wrote: > > pants.ini, line 33 > > <https://reviews.apache.org/r/40299/diff/1/?file=1125123#file1125123line33> > > > > Seems worth a note this is only used by the vagrant provisioning ... or > > add this to `examples/vagrant/aurorabuild.sh`: > > > > ``` > > export PANTS_CONFIG_OVERRIDE="['vagrant.ini']" > > ``` > > > > Path to the override ini is whatever it needs to be, but it need only > > contain the entry you added here. Looks like the site doc is stale - option is singular valued and the name is pluralized - but some doc here: https://pantsbuild.github.io/invoking.html#overlay-ini-files-with-config-overrides But the correct option help is found via `./pants help-advanced` - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40299/#review106438 --- On Nov. 13, 2015, 9:34 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40299/ > ------- > > (Updated Nov. 13, 2015, 9:34 a.m.) > > > Review request for Aurora, Joshua Cohen and John Sirois. > > > Bugs: AURORA-1538 > https://issues.apache.org/jira/browse/AURORA-1538 > > > Repository: aurora > > > Description > --- > > Restore the third_party python repo, needed for mesos.native egg. > > > Diffs > - > > pants.ini b12248c4834fa172b80506b3872c75df666b85cf > > Diff: https://reviews.apache.org/r/40299/diff/ > > > Testing > --- > > I can now build the executor in vagrant. > > > Thanks, > > Bill Farner > >
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
> On Nov. 13, 2015, 9:55 a.m., John Sirois wrote:
> > pants.ini, line 33
> > <https://reviews.apache.org/r/40299/diff/1/?file=1125123#file1125123line33>
> >
> > Seems worth a note this is only used by the vagrant provisioning ... or
> > add this to `examples/vagrant/aurorabuild.sh`:
> >
> > ```
> > export PANTS_CONFIG_OVERRIDE="['vagrant.ini']"
> > ```
> >
> > Path to the override ini is whatever it needs to be, but it need only
> > contain the entry you added here.
>
> John Sirois wrote:
> Looks like the site doc is stale - option is singular valued and the name
> is pluralized - but some doc here:
> https://pantsbuild.github.io/invoking.html#overlay-ini-files-with-config-overrides
> But the correct option help is found via `./pants help-advanced`
>
> Bill Farner wrote:
> That wouldn't really work, as it's needed any time
> `src/main/python/apache/aurora/executor:thermos_executor` is built (more
> specifically - when `3rdparty/python:mesos.native` is included as a dep.
> Seems like a comment is the only sane path forward, right?
SGTM. Looks like more cleanup could happen going forward though since - fwict
- the only way to get that dep right now is:
```
examples/vagrant/provision-dev-cluster.sh: wget -c
https://svn.apache.org/repos/asf/aurora/3rdparty/ubuntu/trusty64/python/mesos.native-${MESOS_VERSION}-py2.7-linux-x86_64.egg
```
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40299/#review106438
---
On Nov. 13, 2015, 9:34 a.m., Bill Farner wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40299/
> -------
>
> (Updated Nov. 13, 2015, 9:34 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and John Sirois.
>
>
> Bugs: AURORA-1538
> https://issues.apache.org/jira/browse/AURORA-1538
>
>
> Repository: aurora
>
>
> Description
> ---
>
> Restore the third_party python repo, needed for mesos.native egg.
>
>
> Diffs
> -
>
> pants.ini b12248c4834fa172b80506b3872c75df666b85cf
>
> Diff: https://reviews.apache.org/r/40299/diff/
>
>
> Testing
> ---
>
> I can now build the executor in vagrant.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 40197: Fix `./pants test src/test/python::` to work out of the box.
> On Nov. 13, 2015, 8:42 a.m., Joshua Cohen wrote: > > pants.ini, lines 27-29 > > <https://reviews.apache.org/r/40197/diff/1/?file=1123612#file1123612line27> > > > > When I went to commit this change, I got a merge conflict applying the > > patch. According to git, these lines are not found in pants.ini on > > [master](https://github.com/apache/aurora/blob/master/pants.ini)? And just > > to be sure, this is not a github mirroring issue, Apache's git shows the > > [same > > thing](https://git-wip-us.apache.org/repos/asf?p=aurora.git;a=blob;f=pants.ini;h=22f6c59dd93c1a4a2ca78c216e1fd3f3f6c8b752;hb=HEAD). Ah yes - makes sense. Had a lot of changes in flight and this one nuked that: https://git1-us-west.apache.org/repos/asf?p=aurora.git;a=commit;h=b4102def I'll rebase and post the new diff. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40197/#review106424 --- On Nov. 11, 2015, 1:30 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40197/ > --- > > (Updated Nov. 11, 2015, 1:30 p.m.) > > > Review request for Aurora, Joshua Cohen and Bill Farner. > > > Bugs: AURORA-547 > https://issues.apache.org/jira/browse/AURORA-547 > > > Repository: aurora > > > Description > --- > > The Aurora python tests cannot currently all be run together; so the ci > script passes --no-fast and devs need to remember to do this too. > Improve the dev experience by defaulting this option. > > build-support/jenkins/build.sh | 2 +- > pants.ini | 7 +++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > > Diffs > - > > build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e > pants.ini 0bd8ec829ae65e2296a122166dea13f315323c9f > > Diff: https://reviews.apache.org/r/40197/diff/ > > > Testing > --- > > Ran into this working https://issues.apache.org/jira/browse/AURORA-547. > After the fix, locally ran `./pants test src/test/python:: -- -v` green. > Previously this would lead to test failures and CPU starvation. > > > Thanks, > > John Sirois > >
Re: Review Request 40197: Fix `./pants test src/test/python::` to work out of the box.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40197/ --- (Updated Nov. 13, 2015, 8:46 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Changes --- Fix `./pants test src/test/python::` to work out of the box. The Aurora python tests cannot currently all be run together; so the ci script passes --no-fast and devs need to remember to do this too. Improve the dev experience by defaulting this option. build-support/jenkins/build.sh | 2 +- pants.ini | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) Bugs: AURORA-547 https://issues.apache.org/jira/browse/AURORA-547 Repository: aurora Description --- The Aurora python tests cannot currently all be run together; so the ci script passes --no-fast and devs need to remember to do this too. Improve the dev experience by defaulting this option. build-support/jenkins/build.sh | 2 +- pants.ini | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) Diffs (updated) - build-support/jenkins/build.sh 7277a64b0cbc2ad43e79a5b0cf3d3a1fae135042 pants.ini 22f6c59dd93c1a4a2ca78c216e1fd3f3f6c8b752 Diff: https://reviews.apache.org/r/40197/diff/ Testing --- Ran into this working https://issues.apache.org/jira/browse/AURORA-547. After the fix, locally ran `./pants test src/test/python:: -- -v` green. Previously this would lead to test failures and CPU starvation. Thanks, John Sirois
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40299/#review106438 --- pants.ini (line 33) <https://reviews.apache.org/r/40299/#comment165172> Seems worth a note this is only used by the vagrant provisioning ... or add this to `examples/vagrant/aurorabuild.sh`: ``` export PANTS_CONFIG_OVERRIDE="['vagrant.ini']" ``` Path to the override ini is whatever it needs to be, but it need only contain the entry you added here. - John Sirois On Nov. 13, 2015, 9:34 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40299/ > --- > > (Updated Nov. 13, 2015, 9:34 a.m.) > > > Review request for Aurora, Joshua Cohen and John Sirois. > > > Bugs: AURORA-1538 > https://issues.apache.org/jira/browse/AURORA-1538 > > > Repository: aurora > > > Description > --- > > Restore the third_party python repo, needed for mesos.native egg. > > > Diffs > - > > pants.ini b12248c4834fa172b80506b3872c75df666b85cf > > Diff: https://reviews.apache.org/r/40299/diff/ > > > Testing > --- > > I can now build the executor in vagrant. > > > Thanks, > > Bill Farner > >
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40299/#review106451 --- Ship it! Ship It! - John Sirois On Nov. 13, 2015, 10:24 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40299/ > --- > > (Updated Nov. 13, 2015, 10:24 a.m.) > > > Review request for Aurora, Joshua Cohen and John Sirois. > > > Bugs: AURORA-1538 > https://issues.apache.org/jira/browse/AURORA-1538 > > > Repository: aurora > > > Description > --- > > Restore the third_party python repo, needed for mesos.native egg. > > > Diffs > - > > pants.ini b12248c4834fa172b80506b3872c75df666b85cf > > Diff: https://reviews.apache.org/r/40299/diff/ > > > Testing > --- > > I can now build the executor in vagrant. > > > Thanks, > > Bill Farner > >
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40299/#review106447 --- Ship it! LGTM mod a pending comment - John Sirois On Nov. 13, 2015, 9:34 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40299/ > --- > > (Updated Nov. 13, 2015, 9:34 a.m.) > > > Review request for Aurora, Joshua Cohen and John Sirois. > > > Bugs: AURORA-1538 > https://issues.apache.org/jira/browse/AURORA-1538 > > > Repository: aurora > > > Description > --- > > Restore the third_party python repo, needed for mesos.native egg. > > > Diffs > - > > pants.ini b12248c4834fa172b80506b3872c75df666b85cf > > Diff: https://reviews.apache.org/r/40299/diff/ > > > Testing > --- > > I can now build the executor in vagrant. > > > Thanks, > > Bill Farner > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/ --- (Updated Nov. 13, 2015, 4:27 p.m.) Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and Zameer Manji. Bugs: AURORA-1532 https://issues.apache.org/jira/browse/AURORA-1532 Repository: aurora Description (updated) --- This upgrades to pants 0.0.58 to pick up the newly split off pants python checks contrib plugin. Release notes are here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 The plugin provides both python checkstyle (`compile.pythonstyle`), and a python eval task (`compile.python-eval`). The `python-eval` is turned off since at least one of the Aurora python targets has files that have side-effects upon import (a repl is started). Now style checks run before compile (and thus before tests) and they benefit from fingerprinting; ie: if you test your changes, those tests will run style checks and when you go to commit, those checks will not be re-run by the commit hook (although files you did not test will still need to be checked). A few production files were fixed up according to style failures coming from: + no space after comment opening '#' + unused variables + mis-aligned hanging closing parens. Diffs - build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 build-support/python/checkstyle-check b2bfc5dd71193a8056828e9af05a4c16965f32a1 pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 src/main/python/apache/aurora/admin/maintenance.py 6d94c923ae37bf6b827519d3505b100af306296b src/main/python/apache/aurora/client/api/__init__.py 6f07a3073a5d422373238619d459fbd09d8adf3d src/main/python/apache/aurora/client/cli/client.py 297fb588808c1eebc32ac3374265ba986dab3436 src/main/python/apache/aurora/client/cli/cron.py 6376fd014f2a4da29442b5c2c7eb36578b503ba3 src/main/python/apache/thermos/core/process.py fe95cb3be01b47616596bd78cb9a919b2e8bd978 src/main/python/apache/thermos/monitoring/process_collector_psutil.py f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 src/test/python/apache/aurora/admin/test_admin.py 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 src/test/python/apache/aurora/admin/util.py 3570407b51613d0a7b4fde8a4794d88b98e150b5 src/test/python/apache/aurora/client/cli/test_task.py 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 Diff: https://reviews.apache.org/r/40310/diff/ Testing --- See the discarded https://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40219/#review106527
---
Replacement RB using official plugin is here:
https://reviews.apache.org/r/40310/
- John Sirois
On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
>
> (Updated Nov. 12, 2015, 1:54 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> ---
>
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal. Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
>
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
>
> build-support/hooks/pre-commit
>| 3 +--
> build-support/jenkins/build.sh
>| 9 +
> build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}
>| 13 ++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/__init__.py}| 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/BUILD} | 18
> +++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12
> +---
>
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
> | 36
> build-support/python/checkstyle
>| 34 --
> build-support/python/checkstyle-check
>| 6 +++---
> pants.ini
>| 43
> +++
> src/main/python/apache/aurora/admin/maintenance.py
>| 2 +-
> src/main/python/apache/aurora/client/api/__init__.py
>| 4 ++--
> src/main/python/apache/aurora/client/cli/client.py
>| 2 +-
> src/main/python/apache/aurora/client/cli/cron.py
>| 2 +-
> src/main/python/apache/thermos/core/process.py
>| 6 +++---
> src/main/python/apache/thermos/monitoring/process_collector_psutil.py
>| 1 -
> src/test/python/apache/aurora/admin/test_admin.py
>| 6 --
> src/test/python/apache/aurora/admin/util.py
>| 2 +-
> src/test/python/apache/aurora/client/cli/test_task.py
>| 3 +--
> 21 files changed, 111 insertions(+), 127 deletions(-)
>
>
> Diffs
> -
>
> build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea
> build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e
> build-support/plugins/3rdparty/python/BUILD PRE-CREATION
> build-support/plugins/src/main/python/apache/__init__.py PRE-CREATION
> build-s
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 10:57 p.m., Bill Farner wrote: > > Heh, took a few rounds to understand what was going on > > here...case-insensitive file system strikes again! > > ``` > > ... > > Making install in lib > > /Library/Developer/CommandLineTools/usr/bin/make install-am > > if test no = no; then \ > > case 'darwin15.0.0' in \ > > darwin[56]*) \ > > need_charset_alias=true ;; \ > > darwin* | cygwin* | mingw* | pw32* | cegcc*) \ > > need_charset_alias=false ;; \ > > *) \ > > need_charset_alias=true ;; \ > > esac ; \ > > else \ > > need_charset_alias=false ; \ > > fi ; \ > > if $need_charset_alias; then \ > > /bin/sh > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/build-aux/install-sh > > -d /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib ; \ > > fi ; \ > > if test -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias; > > then \ > > sed -f ref-add.sed > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > /usr/bin/install -c -m 644 > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > ; \ > > rm -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > else \ > > if $need_charset_alias; then \ > > sed -f ref-add.sed charset.alias > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > /usr/bin/install -c -m 644 > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > ; \ > > rm -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > fi ; \ > > fi > > ../build-aux/install-sh -c -d > > '/Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib' > > mkdir: /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install: > > File exists > > mkdir: /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install: > > Not a directory > > make[5]: *** [install-libLIBRARIES] Error 1 > > make[4]: *** [install-am] Error 2 > > make[3]: *** [install] Error 2 > > make[2]: *** [install-recursive] Error 1 > > make[1]: *** [install] Error 2 > > ``` > > > > bison includes an INSTALL file, and your `--prefix` is `$dist/install` :-P > > John Sirois wrote: > Hrm - serves me right for installing OSX on case-sensitive HFS+. > Fixed; although the AuroraBot will continue to be red due to > https://reviews.apache.org/r/40334/ Alright - rebasing against https://reviews.apache.org/r/40334/ so AuroraBot can go green. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106597 --- On Nov. 16, 2015, 8:13 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 16, 2015, 8:13 a.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > build-support/thrift/.gitignore | 7 --- > build-support/thrift/Makefile | 47 > ++- > 2 files changed, 42 insertions(+), 12 deletions(-) > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40324: Isolate the `third_party/` repo to `mesos.native`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40324/ --- (Updated Nov. 16, 2015, 9:48 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Changes --- Improve documentation of mesos.native gymnastics. 3rdparty/python/requirements.txt | 15 +++ examples/vagrant/provision-dev-cluster.sh | 1 + 2 files changed, 16 insertions(+) Bugs: AURORA-1538 https://issues.apache.org/jira/browse/AURORA-1538 Repository: aurora Description --- This lifts `mesos.interface` and `mesos.native` out of `requirements.txt`, ties their versions together and isolates the custom repository needed by `mesos.native` to its `python_requirement`. 3rdparty/python/BUILD| 26 ++ 3rdparty/python/requirements.txt | 2 -- pants.ini| 5 - 3 files changed, 26 insertions(+), 7 deletions(-) Diffs (updated) - 3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 3rdparty/python/requirements.txt 3a78b754ad8e55308554119e279693053951cc6e examples/vagrant/provision-dev-cluster.sh 4474543bac16ac5af0de1f199e36e8b981d52e04 pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 Diff: https://reviews.apache.org/r/40324/diff/ Testing --- Successfully ran end-to-end with this change using a fresh vagrant image and clean repo via: ``` vagrant destroy && \ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 16, 2015, 8:13 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Fixup BISON_PREFIX to use a unique (case aside) dirent name. build-support/thrift/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Repository: aurora Description --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. build-support/thrift/.gitignore | 7 --- build-support/thrift/Makefile | 47 ++- 2 files changed, 42 insertions(+), 12 deletions(-) Diffs (updated) - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 10:57 p.m., Bill Farner wrote: > > Heh, took a few rounds to understand what was going on > > here...case-insensitive file system strikes again! > > ``` > > ... > > Making install in lib > > /Library/Developer/CommandLineTools/usr/bin/make install-am > > if test no = no; then \ > > case 'darwin15.0.0' in \ > > darwin[56]*) \ > > need_charset_alias=true ;; \ > > darwin* | cygwin* | mingw* | pw32* | cegcc*) \ > > need_charset_alias=false ;; \ > > *) \ > > need_charset_alias=true ;; \ > > esac ; \ > > else \ > > need_charset_alias=false ; \ > > fi ; \ > > if $need_charset_alias; then \ > > /bin/sh > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/build-aux/install-sh > > -d /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib ; \ > > fi ; \ > > if test -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias; > > then \ > > sed -f ref-add.sed > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > /usr/bin/install -c -m 644 > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > ; \ > > rm -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > else \ > > if $need_charset_alias; then \ > > sed -f ref-add.sed charset.alias > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > /usr/bin/install -c -m 644 > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.alias > > ; \ > > rm -f > > /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib/charset.tmp > > ; \ > > fi ; \ > > fi > > ../build-aux/install-sh -c -d > > '/Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install/lib' > > mkdir: /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install: > > File exists > > mkdir: /Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install: > > Not a directory > > make[5]: *** [install-libLIBRARIES] Error 1 > > make[4]: *** [install-am] Error 2 > > make[3]: *** [install] Error 2 > > make[2]: *** [install-recursive] Error 1 > > make[1]: *** [install] Error 2 > > ``` > > > > bison includes an INSTALL file, and your `--prefix` is `$dist/install` :-P Hrm - serves me right for installing OSX on case-sensitive HFS+. Fixed; although the AuroraBot will continue to be red due to https://reviews.apache.org/r/40334/ - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106597 --- On Nov. 15, 2015, 9:13 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 15, 2015, 9:13 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > build-support/thrift/.gitignore | 7 --- > build-support/thrift/Makefile | 47 > ++- > 2 files changed, 42 insertions(+), 12 deletions(-) > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/ --- (Updated Nov. 16, 2015, 10:30 a.m.) Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and Zameer Manji. Changes --- Restore test_admin.py type checks and re-format client/api. src/main/python/apache/aurora/client/api/__init__.py | 7 +-- src/test/python/apache/aurora/admin/test_admin.py| 6 ++ 2 files changed, 11 insertions(+), 2 deletions(-) Bugs: AURORA-1532 https://issues.apache.org/jira/browse/AURORA-1532 Repository: aurora Description --- This upgrades to pants 0.0.58 to pick up the newly split off pants python checks contrib plugin. Release notes are here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 The plugin provides both python checkstyle (`compile.pythonstyle`), and a python eval task (`compile.python-eval`). The `python-eval` is turned off since at least one of the Aurora python targets has files that have side-effects upon import (a repl is started). Now style checks run before compile (and thus before tests) and they benefit from fingerprinting; ie: if you test your changes, those tests will run style checks and when you go to commit, those checks will not be re-run by the commit hook (although files you did not test will still need to be checked). A few production files were fixed up according to style failures coming from: + no space after comment opening '#' + unused variables + mis-aligned hanging closing parens. Diffs (updated) - build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 build-support/python/checkstyle-check b2bfc5dd71193a8056828e9af05a4c16965f32a1 pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 src/main/python/apache/aurora/admin/maintenance.py 6d94c923ae37bf6b827519d3505b100af306296b src/main/python/apache/aurora/client/api/__init__.py 6f07a3073a5d422373238619d459fbd09d8adf3d src/main/python/apache/aurora/client/cli/client.py 297fb588808c1eebc32ac3374265ba986dab3436 src/main/python/apache/aurora/client/cli/cron.py 6376fd014f2a4da29442b5c2c7eb36578b503ba3 src/main/python/apache/thermos/core/process.py fe95cb3be01b47616596bd78cb9a919b2e8bd978 src/main/python/apache/thermos/monitoring/process_collector_psutil.py f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 src/test/python/apache/aurora/admin/test_admin.py 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 src/test/python/apache/aurora/admin/util.py 3570407b51613d0a7b4fde8a4794d88b98e150b5 src/test/python/apache/aurora/client/cli/test_task.py 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 Diff: https://reviews.apache.org/r/40310/diff/ Testing --- See the discarded https://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40324: Isolate the `third_party/` repo to `mesos.native`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40324/ --- (Updated Nov. 16, 2015, 10:42 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1538 https://issues.apache.org/jira/browse/AURORA-1538 Repository: aurora Description --- This lifts `mesos.interface` and `mesos.native` out of `requirements.txt`, ties their versions together and isolates the custom repository needed by `mesos.native` to its `python_requirement`. 3rdparty/python/BUILD| 26 ++ 3rdparty/python/requirements.txt | 2 -- pants.ini| 5 - 3 files changed, 26 insertions(+), 7 deletions(-) Diffs - 3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 3rdparty/python/requirements.txt 3a78b754ad8e55308554119e279693053951cc6e examples/vagrant/provision-dev-cluster.sh 4474543bac16ac5af0de1f199e36e8b981d52e04 pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 Diff: https://reviews.apache.org/r/40324/diff/ Testing --- Successfully ran end-to-end with this change using a fresh vagrant image and clean repo via: ``` vagrant destroy && \ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, John Sirois
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote: > > src/test/python/apache/aurora/admin/test_admin.py, lines 158-160 > > <https://reviews.apache.org/r/40310/diff/2/?file=1125865#file1125865line158> > > > > Why kill this? It seems like the intention was to ensure that set quota > > is called with the correct types, but the assert above still passes if we > > were to do `assert_called_with(role, 24, 4001.0, 6001.0)`, thus the > > explicit type checks were necessary? Ah right - I was thinking this was a tautology but its not, `24.0 == 24` is `True`. This action on my part was triggered by: ``` 10:08:08 00:00 [compile] 10:08:08 00:00 [python-eval] 10:08:08 00:00 [pythonstyle] Invalidated 9 targets. E721:ERROR src/test/python/apache/aurora/admin/test_admin.py:158 do not compare types, use 'isinstance()' | assert type(api.set_quota.call_args[0][1]) == type(float()) ``` I've switched to isinstance which also checks what you want; ie: `isinstance(24.0, int)` fails, etc. > On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote: > > pants.ini, line 19 > > <https://reviews.apache.org/r/40310/diff/2/?file=1125858#file1125858line19> > > > > nit: fix indent? You'll need to give me exact direction, -1 or +N or pull up onto the end of the prior line. There must be some whitespace indent per ConfigParser rules to keep this as a single value. > On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote: > > pants.ini, line 77 > > <https://reviews.apache.org/r/40310/diff/2/?file=1125858#file1125858line77> > > > > or maybe this identation style is intentional/correct? Ah - yes, the indent is intentional / required. The indent is required as explained above, but the amount of indent is a style thing and your call. > On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/api/__init__.py, lines 283-284 > > <https://reviews.apache.org/r/40310/diff/2/?file=1125860#file1125860line283> > > > > alternately this should validate and save the interim var? > > > > return Restarter( > > job_key, > > updater_config, > > ... > > self._scheduler_proxy).restart(instances) I couldn't follow the comment, but did change the style to the one you demonstrated. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/#review106674 --- On Nov. 15, 2015, 9:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40310/ > --- > > (Updated Nov. 15, 2015, 9:02 p.m.) > > > Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and > Zameer Manji. > > > Bugs: AURORA-1532 > https://issues.apache.org/jira/browse/AURORA-1532 > > > Repository: aurora > > > Description > --- > > This upgrades to pants 0.0.58 to pick up the newly split off pants > python checks contrib plugin. Release notes are here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 > > The plugin provides both python checkstyle (`compile.pythonstyle`), and > a python eval task (`compile.python-eval`). The `python-eval` is turned > off since at least one of the Aurora python targets has files that have > side-effects upon import (a repl is started). > > Now style checks run before compile (and thus before tests) and they > benefit from fingerprinting; ie: if you test your changes, those tests > will run style checks and when you go to commit, those checks will not > be re-run by the commit hook (although files you did not test will still > need to be checked). > > A few production files were fixed up according to style failures coming > from: > + no space after comment opening '#' > + unused variables > + mis-aligned hanging closing parens. > > > Diffs > - > > build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea > build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 > build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 > build-support/python/checkstyle-check > b2bfc5dd71193a8056828e9af05a4c16965f32a1 > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > src/main/python/apache/aurora/admin/maintenance.py > 6d94c923ae37bf6b827519d3505b100af306296b > src/main/python/apache/aurora/client/api/__init__.py
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 16, 2015, 1:05 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Fixup BISON_PREFIX to use a unique (case aside) dirent name. build-support/thrift/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Repository: aurora Description --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. build-support/thrift/.gitignore | 7 --- build-support/thrift/Makefile | 47 ++- 2 files changed, 42 insertions(+), 12 deletions(-) Diffs (updated) - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 16, 2015, 3:18 p.m., Zameer Manji wrote: > > build-support/thrift/Makefile, line 28 > > <https://reviews.apache.org/r/40323/diff/6/?file=1127537#file1127537line28> > > > > The ticket says this patch was committed in 0.9.2. Would it be more > > productive to upgrade to 0.9.2+ before complicating our thrift build? > > > > Not a blocker to landing this but I think it would be nice if we didn't > > have to download a patch for our vendored thrit. Sadly 0.9.2 and 0.9.3 are not useable in python, details here: https://issues.apache.org/jira/browse/THRIFT-3388 In short, thrift folks accepted a patch they never should have that poisened python use for any but the vey simplest structs and service methods. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106749 ------- On Nov. 16, 2015, 1:11 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 16, 2015, 1:11 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 16, 2015, 3:18 p.m., Zameer Manji wrote: > > build-support/thrift/Makefile, line 28 > > <https://reviews.apache.org/r/40323/diff/6/?file=1127537#file1127537line28> > > > > The ticket says this patch was committed in 0.9.2. Would it be more > > productive to upgrade to 0.9.2+ before complicating our thrift build? > > > > Not a blocker to landing this but I think it would be nice if we didn't > > have to download a patch for our vendored thrit. > > John Sirois wrote: > Sadly 0.9.2 and 0.9.3 are not useable in python, details here: > https://issues.apache.org/jira/browse/THRIFT-3388 > In short, thrift folks accepted a patch they never should have that > poisened python use for any but the vey simplest structs and service methods. More info on this from the Aurora point of view is here: https://issues.apache.org/jira/browse/AURORA-1083 - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106749 --- On Nov. 16, 2015, 1:11 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 16, 2015, 1:11 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/ --- (Updated Nov. 16, 2015, 1:09 p.m.) Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and Zameer Manji. Changes --- Restore test_admin.py type checks and re-format client/api. src/main/python/apache/aurora/client/api/__init__.py | 7 +-- src/test/python/apache/aurora/admin/test_admin.py| 6 ++ 2 files changed, 11 insertions(+), 2 deletions(-) Bugs: AURORA-1532 https://issues.apache.org/jira/browse/AURORA-1532 Repository: aurora Description --- This upgrades to pants 0.0.58 to pick up the newly split off pants python checks contrib plugin. Release notes are here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 The plugin provides both python checkstyle (`compile.pythonstyle`), and a python eval task (`compile.python-eval`). The `python-eval` is turned off since at least one of the Aurora python targets has files that have side-effects upon import (a repl is started). Now style checks run before compile (and thus before tests) and they benefit from fingerprinting; ie: if you test your changes, those tests will run style checks and when you go to commit, those checks will not be re-run by the commit hook (although files you did not test will still need to be checked). A few production files were fixed up according to style failures coming from: + no space after comment opening '#' + unused variables + mis-aligned hanging closing parens. Diffs (updated) - build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 build-support/python/checkstyle-check b2bfc5dd71193a8056828e9af05a4c16965f32a1 pants.ini d58908ca8356e53c5317fc8fd7531d5285e21fa7 src/main/python/apache/aurora/admin/maintenance.py 6d94c923ae37bf6b827519d3505b100af306296b src/main/python/apache/aurora/client/api/__init__.py 6f07a3073a5d422373238619d459fbd09d8adf3d src/main/python/apache/aurora/client/cli/client.py 297fb588808c1eebc32ac3374265ba986dab3436 src/main/python/apache/aurora/client/cli/cron.py 6376fd014f2a4da29442b5c2c7eb36578b503ba3 src/main/python/apache/thermos/core/process.py fe95cb3be01b47616596bd78cb9a919b2e8bd978 src/main/python/apache/thermos/monitoring/process_collector_psutil.py f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 src/test/python/apache/aurora/admin/test_admin.py 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 src/test/python/apache/aurora/admin/util.py 3570407b51613d0a7b4fde8a4794d88b98e150b5 src/test/python/apache/aurora/client/cli/test_task.py 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 Diff: https://reviews.apache.org/r/40310/diff/ Testing --- See the discarded https://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40220: Modernize the pex venv script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40220/ --- (Updated Nov. 16, 2015, 1:12 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Repository: aurora Description (updated) --- This converts from grabbing the old Twitter python pex to grabbing modern pex to match the version specified in 3rdparty to help keep the pex venv up to date with the codebase dependencies. Diffs - build-support/pex 54e31f11f152cada3809ebd7b2cdec1d2ba12ac9 Diff: https://reviews.apache.org/r/40220/diff/ Testing --- Locally ran this 2x and observed both the proper version (1.1.0) and proper use of the cached venv in the second run: `git clean -fdx build-support && ./build-support/pex --version` Thanks, John Sirois
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 16, 2015, 12:43 p.m., Zameer Manji wrote: > > pants.ini, line 49 > > <https://reviews.apache.org/r/40310/diff/3/?file=1126335#file1126335line49> > > > > Out of curiosity, could we replace isort with this functionality? I > > like the idea of collapsing all of that functionality into pants. You could, but right now the checker is not flexible enough to match the current style. So either a style thrash or else work in pants to make the sorting more flexible (or replaceable with isort rules internally). - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/#review106710 --- On Nov. 16, 2015, 10:30 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40310/ > --- > > (Updated Nov. 16, 2015, 10:30 a.m.) > > > Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and > Zameer Manji. > > > Bugs: AURORA-1532 > https://issues.apache.org/jira/browse/AURORA-1532 > > > Repository: aurora > > > Description > --- > > This upgrades to pants 0.0.58 to pick up the newly split off pants > python checks contrib plugin. Release notes are here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 > > The plugin provides both python checkstyle (`compile.pythonstyle`), and > a python eval task (`compile.python-eval`). The `python-eval` is turned > off since at least one of the Aurora python targets has files that have > side-effects upon import (a repl is started). > > Now style checks run before compile (and thus before tests) and they > benefit from fingerprinting; ie: if you test your changes, those tests > will run style checks and when you go to commit, those checks will not > be re-run by the commit hook (although files you did not test will still > need to be checked). > > A few production files were fixed up according to style failures coming > from: > + no space after comment opening '#' > + unused variables > + mis-aligned hanging closing parens. > > > Diffs > - > > build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea > build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 > build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 > build-support/python/checkstyle-check > b2bfc5dd71193a8056828e9af05a4c16965f32a1 > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > src/main/python/apache/aurora/admin/maintenance.py > 6d94c923ae37bf6b827519d3505b100af306296b > src/main/python/apache/aurora/client/api/__init__.py > 6f07a3073a5d422373238619d459fbd09d8adf3d > src/main/python/apache/aurora/client/cli/client.py > 297fb588808c1eebc32ac3374265ba986dab3436 > src/main/python/apache/aurora/client/cli/cron.py > 6376fd014f2a4da29442b5c2c7eb36578b503ba3 > src/main/python/apache/thermos/core/process.py > fe95cb3be01b47616596bd78cb9a919b2e8bd978 > src/main/python/apache/thermos/monitoring/process_collector_psutil.py > f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 > src/test/python/apache/aurora/admin/test_admin.py > 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 > src/test/python/apache/aurora/admin/util.py > 3570407b51613d0a7b4fde8a4794d88b98e150b5 > src/test/python/apache/aurora/client/cli/test_task.py > 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 > > Diff: https://reviews.apache.org/r/40310/diff/ > > > Testing > --- > > See the discarded https://reviews.apache.org/r/40219/ for the > commit-hook check. This version of that RB engages the same code > and this RB commit was vetted by the same commit-hook. > > > Thanks, > > John Sirois > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/ --- (Updated Nov. 16, 2015, 1:11 p.m.) Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and Zameer Manji. Bugs: AURORA-1532 https://issues.apache.org/jira/browse/AURORA-1532 Repository: aurora Description (updated) --- This installs the ~newly split off pants python checks contrib plugin. Release notes for that were here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 The plugin provides both python checkstyle (`compile.pythonstyle`), and a python eval task (`compile.python-eval`). The `python-eval` is turned off since at least one of the Aurora python targets has files that have side-effects upon import (a repl is started). Now style checks run before compile (and thus before tests) and they benefit from fingerprinting; ie: if you test your changes, those tests will run style checks and when you go to commit, those checks will not be re-run by the commit hook (although files you did not test will still need to be checked). A few production files were fixed up according to style failures coming from: + no space after comment opening '#' + unused variables + mis-aligned hanging closing parens. Diffs - build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 build-support/python/checkstyle-check b2bfc5dd71193a8056828e9af05a4c16965f32a1 pants.ini d58908ca8356e53c5317fc8fd7531d5285e21fa7 src/main/python/apache/aurora/admin/maintenance.py 6d94c923ae37bf6b827519d3505b100af306296b src/main/python/apache/aurora/client/api/__init__.py 6f07a3073a5d422373238619d459fbd09d8adf3d src/main/python/apache/aurora/client/cli/client.py 297fb588808c1eebc32ac3374265ba986dab3436 src/main/python/apache/aurora/client/cli/cron.py 6376fd014f2a4da29442b5c2c7eb36578b503ba3 src/main/python/apache/thermos/core/process.py fe95cb3be01b47616596bd78cb9a919b2e8bd978 src/main/python/apache/thermos/monitoring/process_collector_psutil.py f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 src/test/python/apache/aurora/admin/test_admin.py 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 src/test/python/apache/aurora/admin/util.py 3570407b51613d0a7b4fde8a4794d88b98e150b5 src/test/python/apache/aurora/client/cli/test_task.py 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 Diff: https://reviews.apache.org/r/40310/diff/ Testing --- See the discarded https://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 16, 2015, 1:11 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description (updated) --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. Diffs - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40201: Cleanup thermos_executor test pexes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40201/ --- (Updated Nov. 16, 2015, 1:12 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Brian Wickman, and Zameer Manji. Bugs: AURORA-547 https://issues.apache.org/jira/browse/AURORA-547 Repository: aurora Description (updated) --- Previously these were built to the standard `dist/` dir, now they are dumped to a tmp dir that's cleaned up. Diffs - src/test/python/apache/aurora/executor/test_thermos_executor.py d24c61162e11bb4afef2902ebb031478fe3ed247 src/test/python/apache/aurora/executor/test_thermos_task_runner.py bb998c0b0ac87ba51ec13e490338b00e7f85e8cf Diff: https://reviews.apache.org/r/40201/diff/ Testing --- Green locally: ``` $ rm -rf dist/ && \ ./pants test.pytest --no-fast src/test/python/apache/aurora/executor: ``` And no `dist/` created. Thanks, John Sirois
Re: Review Request 40334: Upgrade to pants `0.0.59` to avoid pytest errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40334/#review106573 --- Right diff2, wrong review - that was intended for https://reviews.apache.org/r/40310/ Fixed diff coming... - John Sirois On Nov. 15, 2015, 4:32 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40334/ > --- > > (Updated Nov. 15, 2015, 4:32 p.m.) > > > Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji. > > > Repository: aurora > > > Description > --- > > The problem is described here: > https://github.com/pantsbuild/pants/issues/2566 > > Changelogs are here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.59 > > pants.ini | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > Diffs > - > > build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea > build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 > build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 > build-support/python/checkstyle-check > b2bfc5dd71193a8056828e9af05a4c16965f32a1 > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > src/main/python/apache/aurora/admin/maintenance.py > 6d94c923ae37bf6b827519d3505b100af306296b > src/main/python/apache/aurora/client/api/__init__.py > 6f07a3073a5d422373238619d459fbd09d8adf3d > src/main/python/apache/aurora/client/cli/client.py > 297fb588808c1eebc32ac3374265ba986dab3436 > src/main/python/apache/aurora/client/cli/cron.py > 6376fd014f2a4da29442b5c2c7eb36578b503ba3 > src/main/python/apache/thermos/core/process.py > fe95cb3be01b47616596bd78cb9a919b2e8bd978 > src/main/python/apache/thermos/monitoring/process_collector_psutil.py > f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 > src/test/python/apache/aurora/admin/test_admin.py > 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 > src/test/python/apache/aurora/admin/util.py > 3570407b51613d0a7b4fde8a4794d88b98e150b5 > src/test/python/apache/aurora/client/cli/test_task.py > 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 > > Diff: https://reviews.apache.org/r/40334/diff/ > > > Testing > --- > > Before the fix stumbled into this: > ``` > $ ./build-support/jenkins/build.sh > ... > 11:22:51 00:00 [test] > 11:22:51 00:00 [run_prep_command] > 11:22:51 00:00 [test] > 11:22:51 00:00 [pytest] > 11:22:51 00:00 [run] > > 11:23:04 00:13 [chroot] > 11:23:11 00:20 [complete] >FAILURE > > Exception message: Could not satisfy all requirements for pytest>=2.6,<2.7: > pytest>=2.6,<2.7, pytest>=2.8.0(from: pytest-timeout) > ``` > > That triggered a bit of a pants release firefight leading > to this RB and one for Medium's mono repo as well as patching > of the fix in at Square. Now > `./build-support/jenkins/build.sh` runs green as well. > > > Thanks, > > John Sirois > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/#review106572 --- This is now rebased against https://reviews.apache.org/r/40334/ in anticipation of that fix landing Monday to prevent test errors. - John Sirois On Nov. 15, 2015, 4:34 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40310/ > --- > > (Updated Nov. 15, 2015, 4:34 p.m.) > > > Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and > Zameer Manji. > > > Bugs: AURORA-1532 > https://issues.apache.org/jira/browse/AURORA-1532 > > > Repository: aurora > > > Description > --- > > This upgrades to pants 0.0.58 to pick up the newly split off pants > python checks contrib plugin. Release notes are here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 > > The plugin provides both python checkstyle (`compile.pythonstyle`), and > a python eval task (`compile.python-eval`). The `python-eval` is turned > off since at least one of the Aurora python targets has files that have > side-effects upon import (a repl is started). > > Now style checks run before compile (and thus before tests) and they > benefit from fingerprinting; ie: if you test your changes, those tests > will run style checks and when you go to commit, those checks will not > be re-run by the commit hook (although files you did not test will still > need to be checked). > > A few production files were fixed up according to style failures coming > from: > + no space after comment opening '#' > + unused variables > + mis-aligned hanging closing parens. > > > Diffs > - > > build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea > build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 > build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 > build-support/python/checkstyle-check > b2bfc5dd71193a8056828e9af05a4c16965f32a1 > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > src/main/python/apache/aurora/admin/maintenance.py > 6d94c923ae37bf6b827519d3505b100af306296b > src/main/python/apache/aurora/client/api/__init__.py > 6f07a3073a5d422373238619d459fbd09d8adf3d > src/main/python/apache/aurora/client/cli/client.py > 297fb588808c1eebc32ac3374265ba986dab3436 > src/main/python/apache/aurora/client/cli/cron.py > 6376fd014f2a4da29442b5c2c7eb36578b503ba3 > src/main/python/apache/thermos/core/process.py > fe95cb3be01b47616596bd78cb9a919b2e8bd978 > src/main/python/apache/thermos/monitoring/process_collector_psutil.py > f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 > src/test/python/apache/aurora/admin/test_admin.py > 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 > src/test/python/apache/aurora/admin/util.py > 3570407b51613d0a7b4fde8a4794d88b98e150b5 > src/test/python/apache/aurora/client/cli/test_task.py > 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 > > Diff: https://reviews.apache.org/r/40310/diff/ > > > Testing > --- > > See the discarded https://reviews.apache.org/r/40219/ for the > commit-hook check. This version of that RB engages the same code > and this RB commit was vetted by the same commit-hook. > > > Thanks, > > John Sirois > >
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 5:29 p.m., Bill Farner wrote:
> > build-support/thrift/.gitignore, line 1
> > <https://reviews.apache.org/r/40323/diff/1/?file=1125751#file1125751line1>
> >
> > How about wildcards so we don't trip over this when upgrading
> > bison/thrift?
Sounds good - done.
> On Nov. 15, 2015, 5:29 p.m., Bill Farner wrote:
> > build-support/thrift/Makefile, line 50
> > <https://reviews.apache.org/r/40323/diff/1/?file=1125752#file1125752line50>
> >
> > On OS X 10.11 i get the following from this line:
> > ```
> > sha256=$(curl -s https://ftp.gnu.org/gnu/bison/bison-2.5.1.tar.gz | tee
> > bison-2.5.1.tar.gz |openssl sha256 | cut -d' ' -f2) && \
> > [ "${sha256}" =
> > "48dc3649231b75ac160d73528000ec89b6cd8d3b87cb7d0713f72ef4610442d4" ] && \
> > tar zxvf bison-2.5.1.tar.gz && \
> > cd bison-2.5.1 && \
> > ./configure
> > --prefix=/Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install
> > && \
> > make clean && make -j4 && \
> > make install
> > openssl:Error: 'sha256' is an invalid command.
> > ```
> >
> > Looks like the command syntax on OS X is `openssl dgst -sha256`, i
> > don't suppose that's supported by your version of openssl?
> >
> > I feel like i've been down this road before, not sure if i've found a
> > universal command.
>
> John Sirois wrote:
> `openssl dgst -sha256` does work for me. I'll see if it works on my OSX
> 10.10.5 machine and in the Vagrant image and report back.
Fixed. Have not tried on my 10.10.5 yet, I'll update with that result later -
Vagrant trusty image like it though.
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40323/#review106577
---
On Nov. 14, 2015, 4:44 p.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40323/
> ---
>
> (Updated Nov. 14, 2015, 4:44 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> ---
>
> This improves the isolation of the thrift build by building a local
> pinned bison dep. For one, this gets thrift 0.9.1 building on Arch
> Linux, a rolling release that is otherwise too modern in its libs to
> successfully build thrift 0.9.1.
>
> Additionally, this change knocks out the checksumming TODO and now
> checks all downloaded tarballs meet their expected hashes. The
> hashes were generated locally after checking the sha1's where
> available.
>
> build-support/thrift/.gitignore | 7 ---
> build-support/thrift/Makefile | 47
> ++-
> 2 files changed, 42 insertions(+), 12 deletions(-)
>
>
> Diffs
> -
>
> build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7
> build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9
>
> Diff: https://reviews.apache.org/r/40323/diff/
>
>
> Testing
> ---
>
> I can now run `./build-support/jenkins/build.sh` green locally.
>
> I also got a hold of an OSX 10.10.5 box and was able to run
> `./build-support/jenkins/build.sh` green on it using this change.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 15, 2015, 7:45 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Improve thrift 0.9.1 build. + Use a more flexible .gitignore with an eye to future thrift upgrades. + Use a more portable sha256 hash command. + Apply a patch that made it into thrift 0.9.2 that improves the robustness of the 0.9.1 build across platforms. build-support/thrift/.gitignore | 6 ++ build-support/thrift/Makefile | 12 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) Repository: aurora Description --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. build-support/thrift/.gitignore | 7 --- build-support/thrift/Makefile | 47 ++- 2 files changed, 42 insertions(+), 12 deletions(-) Diffs (updated) - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40334: Upgrade to pants `0.0.59` to avoid pytest errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40334/#review106574 --- OK - diff 3 restored to normal, just a pants rev bump. Sorry about that! - John Sirois On Nov. 15, 2015, 4:36 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40334/ > --- > > (Updated Nov. 15, 2015, 4:36 p.m.) > > > Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji. > > > Repository: aurora > > > Description > --- > > The problem is described here: > https://github.com/pantsbuild/pants/issues/2566 > > Changelogs are here: > https://pypi.python.org/pypi/pantsbuild.pants/0.0.59 > > pants.ini | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > Diffs > - > > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > > Diff: https://reviews.apache.org/r/40334/diff/ > > > Testing > --- > > Before the fix stumbled into this: > ``` > $ ./build-support/jenkins/build.sh > ... > 11:22:51 00:00 [test] > 11:22:51 00:00 [run_prep_command] > 11:22:51 00:00 [test] > 11:22:51 00:00 [pytest] > 11:22:51 00:00 [run] > > 11:23:04 00:13 [chroot] > 11:23:11 00:20 [complete] >FAILURE > > Exception message: Could not satisfy all requirements for pytest>=2.6,<2.7: > pytest>=2.6,<2.7, pytest>=2.8.0(from: pytest-timeout) > ``` > > That triggered a bit of a pants release firefight leading > to this RB and one for Medium's mono repo as well as patching > of the fix in at Square. Now > `./build-support/jenkins/build.sh` runs green as well. > > > Thanks, > > John Sirois > >
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 5:29 p.m., Bill Farner wrote:
> > build-support/thrift/Makefile, line 50
> > <https://reviews.apache.org/r/40323/diff/1/?file=1125752#file1125752line50>
> >
> > On OS X 10.11 i get the following from this line:
> > ```
> > sha256=$(curl -s https://ftp.gnu.org/gnu/bison/bison-2.5.1.tar.gz | tee
> > bison-2.5.1.tar.gz |openssl sha256 | cut -d' ' -f2) && \
> > [ "${sha256}" =
> > "48dc3649231b75ac160d73528000ec89b6cd8d3b87cb7d0713f72ef4610442d4" ] && \
> > tar zxvf bison-2.5.1.tar.gz && \
> > cd bison-2.5.1 && \
> > ./configure
> > --prefix=/Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install
> > && \
> > make clean && make -j4 && \
> > make install
> > openssl:Error: 'sha256' is an invalid command.
> > ```
> >
> > Looks like the command syntax on OS X is `openssl dgst -sha256`, i
> > don't suppose that's supported by your version of openssl?
> >
> > I feel like i've been down this road before, not sure if i've found a
> > universal command.
`openssl dgst -sha256` does work for me. I'll see if it works on my OSX
10.10.5 machine and in the Vagrant image and report back.
- John
-------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40323/#review106577
---
On Nov. 14, 2015, 4:44 p.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40323/
> ---
>
> (Updated Nov. 14, 2015, 4:44 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> ---
>
> This improves the isolation of the thrift build by building a local
> pinned bison dep. For one, this gets thrift 0.9.1 building on Arch
> Linux, a rolling release that is otherwise too modern in its libs to
> successfully build thrift 0.9.1.
>
> Additionally, this change knocks out the checksumming TODO and now
> checks all downloaded tarballs meet their expected hashes. The
> hashes were generated locally after checking the sha1's where
> available.
>
> build-support/thrift/.gitignore | 7 ---
> build-support/thrift/Makefile | 47
> ++-
> 2 files changed, 42 insertions(+), 12 deletions(-)
>
>
> Diffs
> -
>
> build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7
> build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9
>
> Diff: https://reviews.apache.org/r/40323/diff/
>
>
> Testing
> ---
>
> I can now run `./build-support/jenkins/build.sh` green locally.
>
> I also got a hold of an OSX 10.10.5 box and was able to run
> `./build-support/jenkins/build.sh` green on it using this change.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 40310: Replace Twitter checkstyle with pants checkstyle.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40310/ --- (Updated Nov. 15, 2015, 9:02 p.m.) Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and Zameer Manji. Bugs: AURORA-1532 https://issues.apache.org/jira/browse/AURORA-1532 Repository: aurora Description --- This upgrades to pants 0.0.58 to pick up the newly split off pants python checks contrib plugin. Release notes are here: https://pypi.python.org/pypi/pantsbuild.pants/0.0.58 The plugin provides both python checkstyle (`compile.pythonstyle`), and a python eval task (`compile.python-eval`). The `python-eval` is turned off since at least one of the Aurora python targets has files that have side-effects upon import (a repl is started). Now style checks run before compile (and thus before tests) and they benefit from fingerprinting; ie: if you test your changes, those tests will run style checks and when you go to commit, those checks will not be re-run by the commit hook (although files you did not test will still need to be checked). A few production files were fixed up according to style failures coming from: + no space after comment opening '#' + unused variables + mis-aligned hanging closing parens. Diffs - build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26 build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 build-support/python/checkstyle-check b2bfc5dd71193a8056828e9af05a4c16965f32a1 pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 src/main/python/apache/aurora/admin/maintenance.py 6d94c923ae37bf6b827519d3505b100af306296b src/main/python/apache/aurora/client/api/__init__.py 6f07a3073a5d422373238619d459fbd09d8adf3d src/main/python/apache/aurora/client/cli/client.py 297fb588808c1eebc32ac3374265ba986dab3436 src/main/python/apache/aurora/client/cli/cron.py 6376fd014f2a4da29442b5c2c7eb36578b503ba3 src/main/python/apache/thermos/core/process.py fe95cb3be01b47616596bd78cb9a919b2e8bd978 src/main/python/apache/thermos/monitoring/process_collector_psutil.py f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 src/test/python/apache/aurora/admin/test_admin.py 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 src/test/python/apache/aurora/admin/util.py 3570407b51613d0a7b4fde8a4794d88b98e150b5 src/test/python/apache/aurora/client/cli/test_task.py 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 Diff: https://reviews.apache.org/r/40310/diff/ Testing --- See the discarded https://reviews.apache.org/r/40219/ for the commit-hook check. This version of that RB engages the same code and this RB commit was vetted by the same commit-hook. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 5:29 p.m., Bill Farner wrote:
> > build-support/thrift/Makefile, line 50
> > <https://reviews.apache.org/r/40323/diff/1/?file=1125752#file1125752line50>
> >
> > On OS X 10.11 i get the following from this line:
> > ```
> > sha256=$(curl -s https://ftp.gnu.org/gnu/bison/bison-2.5.1.tar.gz | tee
> > bison-2.5.1.tar.gz |openssl sha256 | cut -d' ' -f2) && \
> > [ "${sha256}" =
> > "48dc3649231b75ac160d73528000ec89b6cd8d3b87cb7d0713f72ef4610442d4" ] && \
> > tar zxvf bison-2.5.1.tar.gz && \
> > cd bison-2.5.1 && \
> > ./configure
> > --prefix=/Users/bill/code/aurora/build-support/thrift/bison-2.5.1/install
> > && \
> > make clean && make -j4 && \
> > make install
> > openssl:Error: 'sha256' is an invalid command.
> > ```
> >
> > Looks like the command syntax on OS X is `openssl dgst -sha256`, i
> > don't suppose that's supported by your version of openssl?
> >
> > I feel like i've been down this road before, not sure if i've found a
> > universal command.
>
> John Sirois wrote:
> `openssl dgst -sha256` does work for me. I'll see if it works on my OSX
> 10.10.5 machine and in the Vagrant image and report back.
>
> John Sirois wrote:
> Fixed. Have not tried on my 10.10.5 yet, I'll update with that result
> later - Vagrant trusty image like it though.
Confirmed diff 3 (forthcoming) works on OSX 10.10.5 / XCode 7.0.1
- John
-------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40323/#review106577
---
On Nov. 15, 2015, 7:45 p.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40323/
> ---
>
> (Updated Nov. 15, 2015, 7:45 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> ---
>
> This improves the isolation of the thrift build by building a local
> pinned bison dep. For one, this gets thrift 0.9.1 building on Arch
> Linux, a rolling release that is otherwise too modern in its libs to
> successfully build thrift 0.9.1.
>
> Additionally, this change knocks out the checksumming TODO and now
> checks all downloaded tarballs meet their expected hashes. The
> hashes were generated locally after checking the sha1's where
> available.
>
> build-support/thrift/.gitignore | 7 ---
> build-support/thrift/Makefile | 47
> ++-
> 2 files changed, 42 insertions(+), 12 deletions(-)
>
>
> Diffs
> -
>
> build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7
> build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9
>
> Diff: https://reviews.apache.org/r/40323/diff/
>
>
> Testing
> ---
>
> I can now run `./build-support/jenkins/build.sh` green locally.
>
> I also got a hold of an OSX 10.10.5 box and was able to run
> `./build-support/jenkins/build.sh` green on it using this change.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 15, 2015, 9:13 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. build-support/thrift/.gitignore | 7 --- build-support/thrift/Makefile | 47 ++- 2 files changed, 42 insertions(+), 12 deletions(-) Diffs (updated) - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 7:57 p.m., Aurora ReviewBot wrote: > > Master (7b494f4) is red with this patch. > > ./build-support/jenkins/build.sh > > > > 02:52:16 00:00 [ivy-imports] > > 02:52:16 00:00 [unpack-jars] > > 02:52:16 00:00 [unpack-jars] > > 02:52:16 00:00 [deferred-sources] > > 02:52:16 00:00 [deferred-sources] > > 02:52:16 00:00 [gen] > > 02:52:16 00:00 [thrift] > > 02:52:16 00:00 [protoc] > > 02:52:16 00:00 [antlr] > > 02:52:16 00:00 [ragel] > > 02:52:16 00:00 [jaxb] > > 02:52:16 00:00 [wire] > > 02:52:16 00:00 [jvm-platform-validate] > > 02:52:16 00:00 [jvm-platform-validate] > > 02:52:16 00:00 [resolve] > > 02:52:16 00:00 [ivy] > > 02:52:16 00:00 [bootstrap-nailgun-server] > > 02:52:17 00:01 [resources] > > 02:52:17 00:01 [prepare] > > 02:52:17 00:01 [services] > > 02:52:17 00:01 [compile] > > 02:52:17 00:01 [compile] > > 02:52:17 00:01 [jvm] > > 02:52:17 00:01 [jvm-compilers] > > 02:52:17 00:01 [zinc-pre] > > 02:52:17 00:01 [zinc-post] > > 02:52:17 00:01 [jvm-dep-check] > > 02:52:17 00:01 [test] > > 02:52:17 00:01 [run_prep_command] > > 02:52:17 00:01 [test] > > 02:52:17 00:01 [pytest] > > 02:52:17 00:01 [run] > > > > 02:52:19 00:03 [chroot] > > 02:52:22 00:06 [complete][31m > >FAILURE[0m > > Exception message: Could not satisfy all requirements for pytest>=2.6,<2.7: > > pytest>=2.6,<2.7, pytest>=2.8.0(from: pytest-timeout) > > > > > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" This failure is unrelated to the thrift fix and in fact occurs after thrift is compiled by CI. The fix for this error is here: https://reviews.apache.org/r/40334/ - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106588 --- On Nov. 15, 2015, 7:45 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 15, 2015, 7:45 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > build-support/thrift/.gitignore | 7 --- > build-support/thrift/Makefile | 47 > ++- > 2 files changed, 42 insertions(+), 12 deletions(-) > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 15, 2015, 9:27 p.m., Aurora ReviewBot wrote: > > Master (7b494f4) is red with this patch. > > ./build-support/jenkins/build.sh > > > > 04:22:02 00:00 [ivy-imports] > > 04:22:02 00:00 [unpack-jars] > > 04:22:02 00:00 [unpack-jars] > > 04:22:02 00:00 [jvm-platform-validate] > > 04:22:02 00:00 [jvm-platform-validate] > > 04:22:02 00:00 [deferred-sources] > > 04:22:02 00:00 [deferred-sources] > > 04:22:02 00:00 [gen] > > 04:22:02 00:00 [thrift] > > 04:22:02 00:00 [protoc] > > 04:22:02 00:00 [antlr] > > 04:22:02 00:00 [ragel] > > 04:22:02 00:00 [jaxb] > > 04:22:02 00:00 [wire] > > 04:22:02 00:00 [resolve] > > 04:22:02 00:00 [ivy] > > 04:22:02 00:00 [bootstrap-nailgun-server] > > 04:22:03 00:01 [resources] > > 04:22:03 00:01 [prepare] > > 04:22:03 00:01 [services] > > 04:22:03 00:01 [compile] > > 04:22:03 00:01 [compile] > > 04:22:03 00:01 [jvm] > > 04:22:03 00:01 [jvm-compilers] > > 04:22:03 00:01 [zinc-pre] > > 04:22:03 00:01 [zinc-post] > > 04:22:03 00:01 [jvm-dep-check] > > 04:22:03 00:01 [test] > > 04:22:03 00:01 [run_prep_command] > > 04:22:03 00:01 [test] > > 04:22:03 00:01 [pytest] > > 04:22:03 00:01 [run] > > > > 04:22:05 00:03 [chroot] > > 04:22:08 00:06 [complete][31m > >FAILURE[0m > > Exception message: Could not satisfy all requirements for pytest>=2.6,<2.7: > > pytest>=2.6,<2.7, pytest>=2.8.0(from: pytest-timeout) > > > > > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" I'll wait until https://reviews.apache.org/r/40334/ is in to update this diff. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106591 --- On Nov. 15, 2015, 9:13 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 15, 2015, 9:13 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > build-support/thrift/.gitignore | 7 --- > build-support/thrift/Makefile | 47 > ++- > 2 files changed, 42 insertions(+), 12 deletions(-) > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 39170: Fix NPE on accessing crons set at impossible dates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39170/#review106566 --- Brice - this would be nice to land. Are you able to put time towards the cleanups suggested by Kevin and style fixes failing the build currently? If so - great. If not, I can brush this up and send out a new RB. - John Sirois On Oct. 9, 2015, 6:41 a.m., Brice Arnould wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39170/ > --- > > (Updated Oct. 9, 2015, 6:41 a.m.) > > > Review request for Aurora and Kevin Sweeney. > > > Bugs: AURORA-1385 > https://issues.apache.org/jira/browse/AURORA-1385 > > > Repository: aurora > > > Description > --- > > This is for AURORA-1385 > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java > 043ba7e6858db28001dfb07ea0c2ddf274a1c755 > > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java > 1ddc4e1946910de798f7f423dd1b19ed56dece15 > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > 13b5c222a0d7b9dd347990e6c09aac09ee566315 > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > 64cbd5b58d9254bb741e20e47165732e52569f70 > > Diff: https://reviews.apache.org/r/39170/diff/ > > > Testing > --- > > Added a specific unit test > > > Thanks, > > Brice Arnould > >
Re: Review Request 40204: Update pants bootstrap script to be agnostic to sed version
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40204/#review106171 --- Ship it! Ship It! - John Sirois On Nov. 11, 2015, 4:39 p.m., Joe Smith wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40204/ > --- > > (Updated Nov. 11, 2015, 4:39 p.m.) > > > Review request for Aurora, John Sirois and Zameer Manji. > > > Repository: aurora > > > Description > --- > > Update pants bootstrap script to be agnostic to sed version > > > Diffs > - > > pants 47097994b1044202aa0d8ce6afb8c2dee2a4c27c > > Diff: https://reviews.apache.org/r/40204/diff/ > > > Testing > --- > > > Thanks, > > Joe Smith > >
Review Request 40208: Eliminate OOB pip install of python deps in CI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40208/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-954 https://issues.apache.org/jira/browse/AURORA-954 Repository: aurora Description --- Under older pants, and thus older pex, distributions were fetched using urllib and were not robust to flaky connections. Now that Aurora is on modern pants and pex, which uses the `requests` library for fetching and includes 5 retries by default, this step should no longer be needed to ensure stable CI runs. build-support/jenkins/build.sh | 7 --- pants.ini | 5 - 2 files changed, 12 deletions(-) Diffs - build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e pants.ini 0bd8ec829ae65e2296a122166dea13f315323c9f Diff: https://reviews.apache.org/r/40208/diff/ Testing --- I still can't quite run `./build-support/jenkins/build.sh` straight up due to https://issues.apache.org/jira/browse/AURORA-1083 but ran the following successfully locally which forces re-download of requirements by pants: ``` $ ./pants clean-all test.pytest --no-fast src/test/python:: ``` Thanks, John Sirois
Review Request 40197: Fix `./pants test src/test/python::` to work out of the box.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40197/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-547 https://issues.apache.org/jira/browse/AURORA-547 Repository: aurora Description --- The Aurora python tests cannot currently all be run together; so the ci script passes --no-fast and devs need to remember to do this too. Improve the dev experience by defaulting this option. build-support/jenkins/build.sh | 2 +- pants.ini | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) Diffs - build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e pants.ini 0bd8ec829ae65e2296a122166dea13f315323c9f Diff: https://reviews.apache.org/r/40197/diff/ Testing --- Ran into this working https://issues.apache.org/jira/browse/AURORA-547. After the fix, locally ran `./pants test src/test/python:: -- -v` green. Previously this would lead to test failures and CPU starvation. Thanks, John Sirois
Review Request 40201: Cleanup thermos_executor test pexes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40201/ --- Review request for Aurora, Kevin Sweeney, Brian Wickman, and Zameer Manji. Bugs: AURORA-547 https://issues.apache.org/jira/browse/AURORA-547 Repository: aurora Description --- Previously these were built to the standard `dist/` dir, now they are dumped to a tmp dir that's cleaned up. src/test/python/apache/aurora/executor/test_thermos_executor.py| 24 +++- src/test/python/apache/aurora/executor/test_thermos_task_runner.py | 15 ++- 2 files changed, 29 insertions(+), 10 deletions(-) Diffs - src/test/python/apache/aurora/executor/test_thermos_executor.py d24c61162e11bb4afef2902ebb031478fe3ed247 src/test/python/apache/aurora/executor/test_thermos_task_runner.py bb998c0b0ac87ba51ec13e490338b00e7f85e8cf Diff: https://reviews.apache.org/r/40201/diff/ Testing --- Green locally: ``` $ rm -rf dist/ && \ ./pants test.pytest --no-fast src/test/python/apache/aurora/executor: ``` And no `dist/` created. Thanks, John Sirois
Re: Review Request 40219: Replace Twitter checkstyle with pants checkstyle.
> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants
> > plugin is the way to go here. Historically we have been very bad at
> > upgrading pants and maintaining it, I'm afraid that if we add a custom
> > plugin here we won't be able to upgrade pants in the future at all.
As you see fit. I will say that the APIs used here are minimal and
historically stable. For example, Medium similarly enables checkstyle as well
as another, non-default-installed plugin, `python-eval`, and they have not had
to touch their plugin since initial install at 0.0.45 in August when it was
released. The other bit to know is pants is locking down to 1.0.0 as the year
closes to isolate more radical changes. There will be a long-term 1.0.0 branch
the Square will be a major user of and a maintainer of with the primary focus
being stability, secondary bugfixes, but ~no new public activity as the pants
community charges on master towards 2.0.
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40219/#review106301
---
On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> ---
>
> (Updated Nov. 12, 2015, 1:54 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> ---
>
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal. Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
>
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
>
> build-support/hooks/pre-commit
>| 3 +--
> build-support/jenkins/build.sh
>| 9 +
> build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}
>| 13 ++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/__init__.py}| 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/__init__.py} | 12
> +---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/BUILD} | 18
> +++---
> build-support/{python/checkstyle-check =>
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12
> +---
>
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
> | 36
> build-support/python/checkstyle
>| 34 --
> build-support/python/checkstyle-check
>| 6 +++---
> pants.ini
>| 43
> +++
> src/main/python/apache/aurora/admin/maintenance.py
>| 2 +-
> src/main/python/apache/aurora/client/api/__init__.py
>| 4 ++--
> src/main/python/apache/aurora/client/cli/client.py
>| 2 +-
> src/main/python/apache/aurora/client/cli/cron.py
>| 2 +-
> src/main/python/apache/thermos/core/process.py
>
Re: Review Request 40323: Improve the isolation in the thrift build.
> On Nov. 16, 2015, 3:18 p.m., Zameer Manji wrote: > > build-support/thrift/Makefile, line 28 > > <https://reviews.apache.org/r/40323/diff/6/?file=1127537#file1127537line28> > > > > The ticket says this patch was committed in 0.9.2. Would it be more > > productive to upgrade to 0.9.2+ before complicating our thrift build? > > > > Not a blocker to landing this but I think it would be nice if we didn't > > have to download a patch for our vendored thrit. > > John Sirois wrote: > Sadly 0.9.2 and 0.9.3 are not useable in python, details here: > https://issues.apache.org/jira/browse/THRIFT-3388 > In short, thrift folks accepted a patch they never should have that > poisened python use for any but the vey simplest structs and service methods. > > John Sirois wrote: > More info on this from the Aurora point of view is here: > https://issues.apache.org/jira/browse/AURORA-1083 I was able to simplify further though for the 2 platforms I'm testing against - Arch and OSX 10.10 - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/#review106749 --- On Nov. 17, 2015, 7:56 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40323/ > --- > > (Updated Nov. 17, 2015, 7:56 a.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > --- > > This improves the isolation of the thrift build by building a local > pinned bison dep. For one, this gets thrift 0.9.1 building on Arch > Linux, a rolling release that is otherwise too modern in its libs to > successfully build thrift 0.9.1. > > Additionally, this change knocks out the checksumming TODO and now > checks all downloaded tarballs meet their expected hashes. The > hashes were generated locally after checking the sha1's where > available. > > > Diffs > - > > build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 > build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 > > Diff: https://reviews.apache.org/r/40323/diff/ > > > Testing > --- > > I can now run `./build-support/jenkins/build.sh` green locally. > > I also got a hold of an OSX 10.10.5 box and was able to run > `./build-support/jenkins/build.sh` green on it using this change. > > > Thanks, > > John Sirois > >
Re: Review Request 40323: Improve the isolation in the thrift build.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40323/ --- (Updated Nov. 17, 2015, 7:56 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Minimize the compilation fix, only the patch is needed. build-support/thrift/.gitignore | 1 - build-support/thrift/Makefile | 29 - 2 files changed, 4 insertions(+), 26 deletions(-) Repository: aurora Description --- This improves the isolation of the thrift build by building a local pinned bison dep. For one, this gets thrift 0.9.1 building on Arch Linux, a rolling release that is otherwise too modern in its libs to successfully build thrift 0.9.1. Additionally, this change knocks out the checksumming TODO and now checks all downloaded tarballs meet their expected hashes. The hashes were generated locally after checking the sha1's where available. Diffs (updated) - build-support/thrift/.gitignore ad6155ebe8671514b5bf5751f250b6cd71c858b7 build-support/thrift/Makefile 51f37a959041536004f83a60d038f2116be8c8a9 Diff: https://reviews.apache.org/r/40323/diff/ Testing --- I can now run `./build-support/jenkins/build.sh` green locally. I also got a hold of an OSX 10.10.5 box and was able to run `./build-support/jenkins/build.sh` green on it using this change. Thanks, John Sirois
Re: Review Request 40391: Introduce a utility class to read executor configurations in JSON.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40391/#review106895 --- Ship it! Ship It! - John Sirois On Nov. 17, 2015, 10:51 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40391/ > --- > > (Updated Nov. 17, 2015, 10:51 a.m.) > > > Review request for Aurora, John Sirois and Zameer Manji. > > > Bugs: AURORA-1376 > https://issues.apache.org/jira/browse/AURORA-1376 > > > Repository: aurora > > > Description > --- > > Another bit-sized chunk towards AURORA-1376. The follow-up will allow an > executor configuration file to be provided on the scheduler command line. > > > Diffs > - > > build.gradle 17dbd2575138880e8ecbbee2b9daf0f1631172c3 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java > 556a3f80236971f6b6cf443d9d1de9394a3aae2d > > src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoaderTest.java > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-missing-field.json > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-thermos-executor.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/40391/diff/ > > > Testing > --- > > > Thanks, > > Bill Farner > >
Review Request 40161: Clarify the commit process for new contributors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40161/ --- Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- A slight emphasis change on the 'Merging' section that favors the new contributor and makes their path to master more front and center. Diffs - CONTRIBUTING.md 5cdb4b1115895d7f1195b4a7135d792003a29a68 Diff: https://reviews.apache.org/r/40161/diff/ Testing --- Eyeballed: `markdown_py CONTRIBUTING.md > CONTRIBUTING.html && open CONTRIBUTING.html` Thanks, John Sirois
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
> On Oct. 29, 2015, 4:15 p.m., Zameer Manji wrote: > > John, would you mind filing a ticket to track the followup work to use the > > built in pants checkstyle? > > Joe Smith wrote: > https://issues.apache.org/jira/browse/AURORA-1532 Thanks Joe. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review104483 --- On Oct. 29, 2015, 4:05 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Oct. 29, 2015, 4:05 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 > build-support/python/make-pycharm-virtualenv > d7bd41835bcd9a8fe62ea522a177bfa7830a897b > build-support/python/update-pants-requirements > 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec > pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 > pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 > src/main/python/apache/aurora/tools/BUILD > e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 > src/main/python/apache/thermos/observer/BUILD > d7eedabc0930711530b45ac98a1159e69d1a0c00 > > Diff: https://reviews.apache.org/r/39784/diff/ > > > Testing > --- > > Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` > > Also generated a pycharm project via: > `./build-support/python/make-pycharm-virtualenv` > Confirmed library source linking worked as did running unit tests > via the IDE. > > Also grepped for pants commands in the repo, found `binary` and `setup-py` > and confirmed these worked. > > > Thanks, > > John Sirois > >
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
> On Oct. 29, 2015, 4:57 p.m., Aurora ReviewBot wrote: > > Master (bcb4774) is red with this patch. > > ./build-support/jenkins/build.sh > > > > 22:56:59 00:00 [unpack-jars] > > 22:56:59 00:00 [unpack-jars] > > 22:56:59 00:00 [deferred-sources] > > 22:56:59 00:00 [deferred-sources] > > 22:56:59 00:00 [jvm-platform-validate] > > 22:56:59 00:00 [jvm-platform-validate] > > 22:56:59 00:00 [gen] > > 22:56:59 00:00 [thrift] > > 22:56:59 00:00 [protoc] > > 22:56:59 00:00 [antlr] > > 22:56:59 00:00 [ragel] > > 22:56:59 00:00 [jaxb] > > 22:57:00 00:01 [wire] > > 22:57:00 00:01 [resolve] > > 22:57:00 00:01 [ivy] > > 22:57:00 00:01 [ivy-bootstrap] > > 22:57:02 00:03 [bootstrap-nailgun-server] > > 22:57:02 00:03 [compile] > > 22:57:02 00:03 [compile] > > 22:57:02 00:03 [jvm] > > 22:57:02 00:03 [jvm-compilers] > > 22:57:02 00:03 [zinc-pre] > > 22:57:02 00:03 [zinc-post] > > 22:57:03 00:04 [jvm-dep-check] > > 22:57:03 00:04 [resources] > > 22:57:03 00:04 [prepare] > > 22:57:03 00:04 [services] > > 22:57:03 00:04 [test] > > 22:57:03 00:04 [run_prep_command] > > 22:57:03 00:04 [test] > > 22:57:03 00:04 [pytest] > > 22:57:03 00:04 [run] > > > > 22:57:07 00:08 [chroot]INFO] Attempting to fetch thrift binary > > from: > > https://dl.bintray.com/pantsbuild/bin/build-support/bin/thrift/linux/x86_64/0.9.1/thrift > > ... > > INFO] Fetched thrift binary from: > > https://dl.bintray.com/pantsbuild/bin/build-support/bin/thrift/linux/x86_64/0.9.1/thrift > > . > > > > 22:57:15 00:16 [complete][31m > >FAILURE[0m > > Exception message: Ambiguous resolvable: thrift > > > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" I need to dig in here, this is unexpected. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review104495 --- On Oct. 29, 2015, 4:05 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Oct. 29, 2015, 4:05 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 > build-support/python/make
Re: Review Request 39784: Upgrade Aurora to pants 0.0.55.
> On Oct. 29, 2015, 4:24 p.m., Zameer Manji wrote: > > Would you also mind updating the commit message to point to the pants > > changelog? Will do. > On Oct. 29, 2015, 4:24 p.m., Zameer Manji wrote: > > BUILD.tools, line 1 > > <https://reviews.apache.org/r/39784/diff/1/?file=1112517#file1112517line1> > > > > Aurora only uses pants for Python code. If we need to specify these JVM > > artifacts for it to work right now, can you please maintain the reference > > to issue 867? Did not mean to nuke that - will restore. > On Oct. 29, 2015, 4:24 p.m., Zameer Manji wrote: > > pants.ini, line 14 > > <https://reviews.apache.org/r/39784/diff/1/?file=1112523#file1112523line14> > > > > Is this comment relevant anymore? If not mind removing it? I'll kill it, its not a lie, but not particularaly useful. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39784/#review104484 ------- On Oct. 29, 2015, 4:05 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39784/ > --- > > (Updated Oct. 29, 2015, 4:05 p.m.) > > > Review request for Aurora, Joe Smith, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1499 > https://issues.apache.org/jira/browse/AURORA-1499 > > > Repository: aurora > > > Description > --- > > This also brings Aurora up to pex 1.1.0 and switches to the pants > setup script; an equivalent to gradlew. Of note, the script is checked > in with chmod 555, its not intended to be edited. > > Although pants now includes checkstyle built in, conversion to use it is > left for follow-on work. > > Also adapt make-pycharm-virtualenv which previously relied on the local > bootstrapping of pants - now hidden away by the pants setup script. > > .pantsversion | 1 - > 3rdparty/python/requirements.txt | 2 +- > BUILD | 25 - > BUILD.tools| 19 -- > build-support/jenkins/build.sh | 2 +- > build-support/pants_requirements.txt | 30 --- > build-support/python/make-pycharm-virtualenv | 11 +- > build-support/python/update-pants-requirements | 34 - > pants | 102 > +++--- > pants.ini | 63 > ++- > src/main/python/apache/aurora/tools/BUILD | 2 +- > src/main/python/apache/thermos/observer/BUILD | 2 +- > 12 files changed, 114 insertions(+), 179 deletions(-) > > > Diffs > - > > .pantsversion 78bae5bb6d254d014e35be0b828497f1509d80bd > 3rdparty/python/requirements.txt 1eeb36dee1a0ebd33999bbb3327338a51cba00d7 > BUILD 7de0c74b03ba609576867ba96885858c0908f2e9 > BUILD.tools 75698a5ded7914a4d22ab7ae769d9ed1576531e4 > build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 > build-support/pants_requirements.txt > fad8da5ce4c03f25554394bc628d4fbb0fe6cd69 > build-support/python/make-pycharm-virtualenv > d7bd41835bcd9a8fe62ea522a177bfa7830a897b > build-support/python/update-pants-requirements > 82f7c5136fad52f4bc2a458beb5f34f0cc7f6cec > pants 6f3526ef76fc37e3215b0673bcb1725d205a1c95 > pants.ini dd4ba668586ee5bd1c888f315429e661adb6a480 > src/main/python/apache/aurora/tools/BUILD > e5ac75838cbe98bbef4e35f6f300b6a6df5e7de5 > src/main/python/apache/thermos/observer/BUILD > d7eedabc0930711530b45ac98a1159e69d1a0c00 > > Diff: https://reviews.apache.org/r/39784/diff/ > > > Testing > --- > > Locally: `./pants test.pytest --no-fast src/test/python:: -- -v` > > Also generated a pycharm project via: > `./build-support/python/make-pycharm-virtualenv` > Confirmed library source linking worked as did running unit tests > via the IDE. > > Also grepped for pants commands in the repo, found `binary` and `setup-py` > and confirmed these worked. > > > Thanks, > > John Sirois > >
Re: Review Request 40324: Isolate the `third_party/` repo to `mesos.native`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40324/#review106548 --- No functional change, I just promised a follow-up to https://reviews.apache.org/r/40299/ with another approach and this is it. This is a take-it or leave it. It certainly could be see as obscuring if folks are used to looking in pants.ini for a custom repos setup. - John Sirois On Nov. 14, 2015, 5:18 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40324/ > --- > > (Updated Nov. 14, 2015, 5:18 p.m.) > > > Review request for Aurora, Joshua Cohen and Bill Farner. > > > Bugs: AURORA-1538 > https://issues.apache.org/jira/browse/AURORA-1538 > > > Repository: aurora > > > Description > --- > > This lifts `mesos.interface` and `mesos.native` out of > `requirements.txt`, ties their versions together and isolates the > custom repository needed by `mesos.native` to its `python_requirement`. > > 3rdparty/python/BUILD| 26 ++ > 3rdparty/python/requirements.txt | 2 -- > pants.ini| 5 - > 3 files changed, 26 insertions(+), 7 deletions(-) > > > Diffs > - > > 3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 > 3rdparty/python/requirements.txt 3a78b754ad8e55308554119e279693053951cc6e > pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 > > Diff: https://reviews.apache.org/r/40324/diff/ > > > Testing > --- > > Successfully ran end-to-end with this change using a fresh vagrant > image and clean repo via: > ``` > vagrant destroy && \ > bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > ``` > > > Thanks, > > John Sirois > >
Re: Review Request 40299: Restore the third_party python repo, needed for mesos.native egg.
> On Nov. 13, 2015, 9:55 a.m., John Sirois wrote:
> > pants.ini, line 33
> > <https://reviews.apache.org/r/40299/diff/1/?file=1125123#file1125123line33>
> >
> > Seems worth a note this is only used by the vagrant provisioning ... or
> > add this to `examples/vagrant/aurorabuild.sh`:
> >
> > ```
> > export PANTS_CONFIG_OVERRIDE="['vagrant.ini']"
> > ```
> >
> > Path to the override ini is whatever it needs to be, but it need only
> > contain the entry you added here.
>
> John Sirois wrote:
> Looks like the site doc is stale - option is singular valued and the name
> is pluralized - but some doc here:
> https://pantsbuild.github.io/invoking.html#overlay-ini-files-with-config-overrides
> But the correct option help is found via `./pants help-advanced`
>
> Bill Farner wrote:
> That wouldn't really work, as it's needed any time
> `src/main/python/apache/aurora/executor:thermos_executor` is built (more
> specifically - when `3rdparty/python:mesos.native` is included as a dep.
> Seems like a comment is the only sane path forward, right?
>
> John Sirois wrote:
> SGTM. Looks like more cleanup could happen going forward though since -
> fwict - the only way to get that dep right now is:
> ```
> examples/vagrant/provision-dev-cluster.sh: wget -c
> https://svn.apache.org/repos/asf/aurora/3rdparty/ubuntu/trusty64/python/mesos.native-${MESOS_VERSION}-py2.7-linux-x86_64.egg
> ```
>
> John Sirois wrote:
> Looks relevant: https://issues.apache.org/jira/browse/AURORA-972
>
> John Sirois wrote:
> I may follow-up with another approach now that I know to test the vagrant
> bit oob from CI. I think this can be localized to the 3rdparty/python/BUILD.
That other approach is over here: https://reviews.apache.org/r/40324/
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40299/#review106438
---
On Nov. 13, 2015, 10:24 a.m., Bill Farner wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40299/
> ---
>
> (Updated Nov. 13, 2015, 10:24 a.m.)
>
>
> Review request for Aurora, Joshua Cohen and John Sirois.
>
>
> Bugs: AURORA-1538
> https://issues.apache.org/jira/browse/AURORA-1538
>
>
> Repository: aurora
>
>
> Description
> ---
>
> Restore the third_party python repo, needed for mesos.native egg.
>
>
> Diffs
> -
>
> pants.ini b12248c4834fa172b80506b3872c75df666b85cf
>
> Diff: https://reviews.apache.org/r/40299/diff/
>
>
> Testing
> ---
>
> I can now build the executor in vagrant.
>
>
> Thanks,
>
> Bill Farner
>
>
Review Request 40324: Isolate the `third_party/` repo to `mesos.native`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40324/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1538 https://issues.apache.org/jira/browse/AURORA-1538 Repository: aurora Description --- This lifts `mesos.interface` and `mesos.native` out of `requirements.txt`, ties their versions together and isolates the custom repository needed by `mesos.native` to its `python_requirement`. 3rdparty/python/BUILD| 26 ++ 3rdparty/python/requirements.txt | 2 -- pants.ini| 5 - 3 files changed, 26 insertions(+), 7 deletions(-) Diffs - 3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 3rdparty/python/requirements.txt 3a78b754ad8e55308554119e279693053951cc6e pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0 Diff: https://reviews.apache.org/r/40324/diff/ Testing --- Successfully ran end-to-end with this change using a fresh vagrant image and clean repo via: ``` vagrant destroy && \ bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, John Sirois
Review Request 40786: Replace manual Forwarding* with `@Forward`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40786/
---
Review request for Aurora, Bill Farner and Zameer Manji.
Repository: aurora
Description
---
This is a bit of a straw man. While working on
https://issues.apache.org/jira/browse/AURORA-987 I found the need for
`javapoet` and this project spun off the side.
See the `@Forward` README here for more info: https://github.com/perkuno/forward
Although I think the end result is desirable, this change does add a
dependency on a personal project. I fancied up the presentation with a
custom org, but its still a personal project. I'd be happy enough to
move the `@Forward` code over to aurora, but it would require a seperate
gradle module to ensure the annotation processor is compiled ahead of
the main module that would use it.
So kick the tires and let me know what you think. The artifact will be
official on maven central within a few hours of this ticket being
resolved: https://issues.sonatype.org/browse/OSSRH-19139, at which point
the custom maven repo in `build.gradle` could go and the rev could be
bumped to `1.0.0`
build.gradle
| 6 +-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
| 185 --
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
| 13 ++-
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
| 309 --
src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
| 5 +-
5 files changed, 20 insertions(+), 498 deletions(-)
Diffs
-
build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
b8bd9185cacc6b113b64a13a1b670fac202c795e
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
a6769e62d61b2851db055e98ee254f707a91d208
src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
1415f0cacc694aa7cf0d25e836e764a96fbb8ae2
Diff: https://reviews.apache.org/r/40786/diff/
Testing
---
Green locally `./build-support/jenkins/build.sh`.
An example of the generated code:
```java
@Generated("uno.perk.forward.apt.Processor")
class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
}
@Override
public Response getRoleSummary() throws TException {
return this.annotatedAuroraAdmin.getRoleSummary();
}
...
```
Thanks,
John Sirois
Review Request 41140: Revert "Replace manual Forwarding* with `@Forward`."
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41140/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-1146 https://issues.apache.org/jira/browse/AURORA-1146 Repository: aurora Description --- This reverts commit 0c98e8a82177f39534db3a49162582aeb1739728. Forward annotation processing was not being handled well by either the gradle idea task nor the Idea gradle plugin (compile works but test debugging in the obvious way from the IDE does not). build.gradle | 3 +- src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java | 185 +++ src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java | 13 +-- src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java | 266 ++ src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java | 5 +- 5 files changed, 455 insertions(+), 17 deletions(-) Diffs - build.gradle fcf8039da96564e8e9a75628b924325616a554a9 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d34f3694a1e47dc4fc15b65f2ae66a1b15abbb4 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 0547b1acf0c1fc09898fe05777f45cbaa169eea7 Diff: https://reviews.apache.org/r/41140/diff/ Testing --- Green: `./gradlew -Pq build` Also confirmd `./gradlew idea` produced a project I could build from within IDEA and debug tests with. Thanks, John Sirois
Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40786/
---
(Updated Dec. 7, 2015, 2:55 p.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Changes
---
Update to forward-1.0.0 on maven central.
build.gradle | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
Repository: aurora
Description
---
This is a bit of a straw man. While working on
https://issues.apache.org/jira/browse/AURORA-987 I found the need for
`javapoet` and this project spun off the side.
See the `@Forward` README here for more info: https://github.com/perkuno/forward
Although I think the end result is desirable, this change does add a
dependency on a personal project. I fancied up the presentation with a
custom org, but its still a personal project. I'd be happy enough to
move the `@Forward` code over to aurora, but it would require a seperate
gradle module to ensure the annotation processor is compiled ahead of
the main module that would use it.
So kick the tires and let me know what you think.
build.gradle
| 6 +-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
| 185 --
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
| 13 ++-
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
| 309 --
src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
| 5 +-
5 files changed, 20 insertions(+), 498 deletions(-)
Diffs (updated)
-
build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
b8bd9185cacc6b113b64a13a1b670fac202c795e
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503
src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
2de178302d3c9aa9a7b23a9eb7ecb6e2f3b40819
src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
1415f0cacc694aa7cf0d25e836e764a96fbb8ae2
Diff: https://reviews.apache.org/r/40786/diff/
Testing
---
Green locally `./build-support/jenkins/build.sh`.
An example of the generated code:
```java
@Generated("uno.perk.forward.apt.Processor")
class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
}
@Override
public Response getRoleSummary() throws TException {
return this.annotatedAuroraAdmin.getRoleSummary();
}
...
```
Thanks,
John Sirois
Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.
> On Dec. 7, 2015, 12:29 p.m., Zameer Manji wrote:
> > I tried to commit this on master and applying the patch failed. John, can
> > you please rebase this?
Should be all good now.
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40786/#review109183
---
On Dec. 7, 2015, 2:55 p.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
>
> (Updated Dec. 7, 2015, 2:55 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> ---
>
> This is a bit of a straw man. While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
>
> See the `@Forward` README here for more info:
> https://github.com/perkuno/forward
>
> Although I think the end result is desirable, this change does add a
> dependency on a personal project. I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
>
> So kick the tires and let me know what you think.
>
> build.gradle
> | 6 +-
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
> | 185 --
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
> | 13 ++-
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
> | 309 --
>
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
> | 5 +-
> 5 files changed, 20 insertions(+), 498 deletions(-)
>
>
> Diffs
> -
>
> build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6
> src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
> b8bd9185cacc6b113b64a13a1b670fac202c795e
>
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503
> src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
> 2de178302d3c9aa9a7b23a9eb7ecb6e2f3b40819
>
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2
>
> Diff: https://reviews.apache.org/r/40786/diff/
>
>
> Testing
> ---
>
> Green locally `./build-support/jenkins/build.sh`.
>
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
> protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
>
> MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
> }
>
> @Override
> public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
> }
> ...
> ```
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 41453: HTTP server cleanup - shut down the server after unit tests, remove jetty bug workaround.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41453/#review110725 --- Drive-by src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java (line 148) <https://reviews.apache.org/r/41453/#comment170750> It looks like this can be an NPE without advanced knowledge of how the base class is used in practice, complicated by the fact the field is set inside a try - seems the setting could be done outside the try, but ... src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java (line 156) <https://reviews.apache.org/r/41453/#comment170749> The Optional indirection (+ volatile bit) seems a bit odd - the startupService is never absent. How about just keep the variable local and non-Optional and on the next line add the stopAsync as a tear down? - John Sirois On Dec. 16, 2015, 11:19 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41453/ > --- > > (Updated Dec. 16, 2015, 11:19 a.m.) > > > Review request for Aurora and Joshua Cohen. > > > Repository: aurora > > > Description > --- > > HTTP server cleanup - shut down the server after unit tests, remove jetty bug > workaround. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 838bfc9ed6242e777d81a17b337f342b7bea72ec > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 5768481005ef505d6c397101d8cc005af9d6815f > > Diff: https://reviews.apache.org/r/41453/diff/ > > > Testing > --- > > > Thanks, > > Bill Farner > >
Re: Review Request 41453: HTTP server cleanup - shut down the server after unit tests, remove jetty bug workaround.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41453/#review110739 --- Ship it! Ship It! - John Sirois On Dec. 16, 2015, 12:53 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41453/ > --- > > (Updated Dec. 16, 2015, 12:53 p.m.) > > > Review request for Aurora and Joshua Cohen. > > > Repository: aurora > > > Description > --- > > HTTP server cleanup - shut down the server after unit tests, remove jetty bug > workaround. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 838bfc9ed6242e777d81a17b337f342b7bea72ec > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 5768481005ef505d6c397101d8cc005af9d6815f > > Diff: https://reviews.apache.org/r/41453/diff/ > > > Testing > --- > > > Thanks, > > Bill Farner > >
Re: Review Request 41226: Handling task event race in updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41226/#review110696 --- Ship it! Master (fb8155d) is green with this patch. true I will refresh this build result if you post a review containing "@ReviewBot retry" - John Sirois On Dec. 11, 2015, 1:33 a.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41226/ > --- > > (Updated Dec. 11, 2015, 1:33 a.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-1506 and AURORA-1507 > https://issues.apache.org/jira/browse/AURORA-1506 > https://issues.apache.org/jira/browse/AURORA-1507 > > > Repository: aurora > > > Description > --- > > Task event processing delays may generate duplicate evaluation results in the > `InstanceActionHandler`. This leads to occasional stack traces in the log but > is generally benign as `JobUpdateEventSubscriber` catches these errors. > > The easiest fix is to check for the task existence before attempting to > kill/add instance. > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java > d8686f1f1b53e5ff2791663489e24c342503831e > src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java > 0583a63a175880355a7296ebd1c6e6fb5dc99f38 > src/test/java/org/apache/aurora/scheduler/updater/KillTaskTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/41226/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 41368: Remove the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41368/#review110695 --- Ship it! Master (fb8155d) is green with this patch. true I will refresh this build result if you post a review containing "@ReviewBot retry" - John Sirois On Dec. 14, 2015, 10:36 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41368/ > --- > > (Updated Dec. 14, 2015, 10:36 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Zameer Manji. > > > Bugs: AURORA-785 > https://issues.apache.org/jira/browse/AURORA-785 > > > Repository: aurora > > > Description > --- > > ``` > NEWS | 4 + > docs/hooks.md | 2 - > .../python/apache/aurora/client/api/__init__.py| 24 - > .../python/apache/aurora/client/api/updater.py | 720 - > src/main/python/apache/aurora/client/base.py | 19 - > src/main/python/apache/aurora/client/cli/jobs.py | 115 --- > .../apache/aurora/client/hooks/hooked_api.py | 14 - > src/test/python/apache/aurora/client/api/BUILD | 12 - > .../apache/aurora/client/api/test_updater.py | 899 > - > src/test/python/apache/aurora/client/cli/BUILD | 13 - > .../apache/aurora/client/cli/test_cancel_update.py | 62 -- > .../python/apache/aurora/client/cli/test_update.py | 528 > .../apache/aurora/client/hooks/test_hooked_api.py | 5 +- > .../aurora/client/hooks/test_non_hooked_api.py | 15 +- > src/test/python/apache/aurora/client/test_base.py | 8 - > .../sh/org/apache/aurora/e2e/test_end_to_end.sh| 8 - > 16 files changed, 11 insertions(+), 2437 deletions(-) > ``` > > > Diffs > - > > NEWS 99531a3ffd98acb08b9bee9c44a1553fba4b56a6 > docs/hooks.md 63dbdb91d014b65f129cd6b28bdb4c563f00ead3 > src/main/python/apache/aurora/client/api/__init__.py > 1514394829cfd8d76d6db5cdfd7c6593f1193b29 > src/main/python/apache/aurora/client/api/updater.py > acbce21e991981a2e85c1a00a68e57d88c5509bb > src/main/python/apache/aurora/client/base.py > 91c276b09500d4eb81368f0b4bd482f0177b5e53 > src/main/python/apache/aurora/client/cli/jobs.py > 46948497f59c9c4d91fdefd8038cff138b9aeb0f > src/main/python/apache/aurora/client/hooks/hooked_api.py > 7fc0b7103e8968af8a9288b698829f31c00e6b32 > src/test/python/apache/aurora/client/api/BUILD > 2756912af8b0716417efa992201bf72ca736a653 > src/test/python/apache/aurora/client/api/test_updater.py > 5f54d22401a5eec0027b156ebaf50114221da8bd > src/test/python/apache/aurora/client/cli/BUILD > 1b14e8c299360f236412c8d9b5e2e143f755f4e0 > src/test/python/apache/aurora/client/cli/test_cancel_update.py > d4fc049ebef5745b7a749a74005be962c63ae2ea > src/test/python/apache/aurora/client/cli/test_update.py > 787396e8da298fab736d4aa8d2b436a68ebee27b > src/test/python/apache/aurora/client/hooks/test_hooked_api.py > 4d8ebabb3a1dcf26dd491b3bb73c0057d6a8ac97 > src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py > 78dffa5bbf065dcae61ae672c928f658b7400522 > src/test/python/apache/aurora/client/test_base.py > 8c880081620a8360bb2b7f0d854c7708c14cae7a > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > d7c61e26428c05d6b2f212e2bb092af38ef6aeec > > Diff: https://reviews.apache.org/r/41368/diff/ > > > Testing > --- > > Test suite + end-to-end tests > > > Thanks, > > Bill Farner > >
Re: Review Request 30695: Implements log rotation in the Thermos runner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30695/#review110697 --- Ship it! Master (fb8155d) is green with this patch. true I will refresh this build result if you post a review containing "@ReviewBot retry" - John Sirois On Nov. 25, 2015, 5 p.m., George Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30695/ > --- > > (Updated Nov. 25, 2015, 5 p.m.) > > > Review request for Aurora, Bill Farner and Brian Wickman. > > > Bugs: AURORA-95 > https://issues.apache.org/jira/browse/AURORA-95 > > > Repository: aurora > > > Description > --- > > Implements log rotation in the Thermos runner. > > > Diffs > - > > docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py > b3e8bf1e924999306e0b8a1314273b22c51028e7 > src/main/python/apache/aurora/executor/thermos_task_runner.py > 14e8b4bd539d2c295582d93fa01b5613345c1758 > src/main/python/apache/thermos/config/schema_base.py > f9143cc1b83143d6147f59d90c79435d055d0518 > src/main/python/apache/thermos/core/process.py > fe95cb3be01b47616596bd78cb9a919b2e8bd978 > src/main/python/apache/thermos/core/runner.py > f949f279a071c6464b026749f51afc776102f2aa > src/main/python/apache/thermos/runner/thermos_runner.py > bd8cf7f4cda54b6be72dad64f9446eedeb132211 > src/test/python/apache/thermos/core/test_process.py > 5e6ad2fca616b840299bd9ca1614c82c5c39e992 > > Diff: https://reviews.apache.org/r/30695/diff/ > > > Testing > --- > > ./pants test src/test/python/apache/thermos/core:all > > > Thanks, > > George Sirois > >
Re: Review Request 41428: Refactoring HealthCheckConfig into separate structs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41428/#review110692 --- Ship it! Master (fb8155d) is green with this patch. true I will refresh this build result if you post a review containing "@ReviewBot retry" - John Sirois On Dec. 16, 2015, 7:15 a.m., Dmitriy Shirchenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41428/ > --- > > (Updated Dec. 16, 2015, 7:15 a.m.) > > > Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1562 > https://issues.apache.org/jira/browse/AURORA-1562 > > > Repository: aurora > > > Description > --- > > Refactoring HealthCheckConfig into separate structs > > > Diffs > - > > docs/configuration-reference.md 07149c7f49371e39b70e8744d60affeffd73c77f > src/main/python/apache/aurora/client/config.py > 161c3627db0afa8fdd8afff5abf6a94556f53180 > src/main/python/apache/aurora/config/schema/base.py > e752482b95ae6377d0cf12cf0ecf0fa60e2a5d46 > src/main/python/apache/aurora/executor/common/health_checker.py > cba4e8c879fae2b9f56a36bf8fa4b702163667d1 > src/test/python/apache/aurora/client/test_config.py > 8fd112fd8a10120f57324d8efec22a009b93404b > src/test/python/apache/aurora/executor/common/test_health_checker.py > 8561abc3e7df8803785b70064bb76553d3210399 > src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora > 37f2e9c6ebc4a4bbd6089e821420e5a02179c6b9 > > Diff: https://reviews.apache.org/r/41428/diff/ > > > Testing > --- > > Added unit tests. > Changed end to end test. > > > Thanks, > > Dmitriy Shirchenko > >
Review Request 41331: Upgrade to pants 0.0.64 and pex 1.1.1.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41331/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1554 https://issues.apache.org/jira/browse/AURORA-1554 Repository: aurora Description --- CHANGELOGs are here: + http://pantsbuild.github.io/changelog.html + https://pypi.python.org/pypi/pex/1.1.1 The pants upgrade ends the deprecation cycle for the `python_test_suite` aggregator target; so these are all switched to the reccomended `target` aggregator target. The pex upgrade brings in support for a global `/etc/pexrc` which could be desirable for control over the executor components. 3rdparty/python/requirements.txt| 2 +- pants.ini | 2 +- src/test/python/BUILD | 2 +- src/test/python/apache/aurora/BUILD | 2 +- src/test/python/apache/aurora/admin/BUILD | 2 +- src/test/python/apache/aurora/client/BUILD | 2 +- src/test/python/apache/aurora/client/api/BUILD | 2 +- src/test/python/apache/aurora/client/cli/BUILD | 2 +- src/test/python/apache/aurora/client/hooks/BUILD| 2 +- src/test/python/apache/aurora/common/BUILD | 2 +- src/test/python/apache/aurora/config/BUILD | 2 +- src/test/python/apache/aurora/executor/BUILD| 6 +++--- src/test/python/apache/aurora/executor/bin/BUILD| 2 +- src/test/python/apache/aurora/executor/common/BUILD | 2 +- src/test/python/apache/aurora/tools/BUILD | 2 +- src/test/python/apache/thermos/BUILD| 2 +- src/test/python/apache/thermos/common/BUILD | 2 +- src/test/python/apache/thermos/config/BUILD | 2 +- src/test/python/apache/thermos/core/BUILD | 8 src/test/python/apache/thermos/monitoring/BUILD | 2 +- src/test/python/apache/thermos/observer/BUILD | 2 +- src/test/python/apache/thermos/observer/http/BUILD | 2 +- 22 files changed, 27 insertions(+), 27 deletions(-) Diffs - 3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 pants.ini 1e19d9af38bc2ec362aa2130cc37d1532454ceee src/test/python/BUILD 34d59a58758ef41ca42fd4944c953bedf4205def src/test/python/apache/aurora/BUILD 7db8b0e3a798a8013ebbcddb73f49bdd5d4373d8 src/test/python/apache/aurora/admin/BUILD d4010458b0580e1a70ee7d5262b2427d25068e09 src/test/python/apache/aurora/client/BUILD 84c5c845d9b1c8078ae8b47242d0f2ffc00ef6dc src/test/python/apache/aurora/client/api/BUILD c375ae98c3fe3041ab02ac2cca0b82428128c0cf src/test/python/apache/aurora/client/cli/BUILD 36fd98a2f3b47630912a3e20f1d34724d508d92d src/test/python/apache/aurora/client/hooks/BUILD ae97ebf2dd77661a8fa600b520c210c239a304ea src/test/python/apache/aurora/common/BUILD 2556c32842b3cf7040cb3c41172a0d9c365cb649 src/test/python/apache/aurora/config/BUILD 92bc6800bc4ac904df9bca8d6d2a784ea243c190 src/test/python/apache/aurora/executor/BUILD 1c973753e65f05e2d25846de9bd693dcd78c86e4 src/test/python/apache/aurora/executor/bin/BUILD c3b538c026de649c442f3d3e8eef653636504830 src/test/python/apache/aurora/executor/common/BUILD 7dafebfd1bb2ab50691df2fbed0dcb8761edff28 src/test/python/apache/aurora/tools/BUILD e676aff357e48fe2c706d74c5b3d8b0a1af5f2dd src/test/python/apache/thermos/BUILD 821307a2824b9f3ebdb2b1da9ecedb5ed46cc28e src/test/python/apache/thermos/common/BUILD 7813dbcdb5d430d4f93cd8e3cea9c63cb625d9a4 src/test/python/apache/thermos/config/BUILD cee0f05246cebc3ee3d02d0227c2262082b0ef89 src/test/python/apache/thermos/core/BUILD 53cc575fb036690b6b361f66c75c11c4362cf4e9 src/test/python/apache/thermos/monitoring/BUILD b8d6d2fbc77c99e4e1739dae0582dd01ba9a5e1b src/test/python/apache/thermos/observer/BUILD 34475d27228ac5f40bcd1289959ee3c13ec28bb0 src/test/python/apache/thermos/observer/http/BUILD bdead2cf04079c583de62854235249020585e145 Diff: https://reviews.apache.org/r/41331/diff/ Testing --- Locally green: `./build-support/jenkins/build.sh` Thanks, John Sirois
Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41528/ --- (Updated Dec. 17, 2015, 1:34 p.m.) Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji. Changes --- Fix several typos. src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java | 2 +- src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java | 2 +- src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) Bugs: AURORA-1385 https://issues.apache.org/jira/browse/AURORA-1385 Repository: aurora Description --- Previously, the fact `null` could be returned by Quartz when calculating the next run was not taken into account. Now The CronPredictor interface makes this possibility manifest with an Optional result. Code that uses the CronPredictor is adjusted and tests are added. NB: This code is as first proposed here by Brice Arnould with small changes: https://reviews.apache.org/r/39170/ src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java | 10 -- src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java | 8 +--- src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java | 13 - src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java | 23 +++ src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java | 35 --- 5 files changed, 72 insertions(+), 17 deletions(-) Diffs (updated) - src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 043ba7e6858db28001dfb07ea0c2ddf274a1c755 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 1ddc4e1946910de798f7f423dd1b19ed56dece15 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java b85f4abd59ef64264fb089527ad42b9ceee7f8d6 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6efe03fe4841cf1275e2ee0c7cc1b9576540f34e Diff: https://reviews.apache.org/r/41528/diff/ Testing --- Green locally: `./gradlew -Pq build` Thanks, John Sirois
Re: Review Request 41521: Force Windows to always use Unix line endings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41521/#review111002 --- Ship it! Ship It! - John Sirois On Dec. 17, 2015, 11:18 a.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41521/ > --- > > (Updated Dec. 17, 2015, 11:18 a.m.) > > > Review request for Aurora and John Sirois. > > > Bugs: AURORA-1354 > https://issues.apache.org/jira/browse/AURORA-1354 > > > Repository: aurora > > > Description > --- > > Since everything is run inside an Ubuntu VM, we want to ensure the line > endings don't get auto-translated. > > > Diffs > - > > .gitattributes a5ab8cf17a14d2527e2a765ff94c3a74e719e0f4 > > Diff: https://reviews.apache.org/r/41521/diff/ > > > Testing > --- > > Applied settings to a checkout on a windows box and verified I was able to > provision a VM w/ Vagrant and connect to the scheduler there. > > > Thanks, > > Joshua Cohen > >
Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41528/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1385 https://issues.apache.org/jira/browse/AURORA-1385 Repository: aurora Description --- Previously, the fact `null` could be returned by Quartz when calculating the next run was not taken into account. Now The CronPredictor interface makes this possibility manifest with an Optional result. Code that uses the CronPredictor is adjusted and tests are added. NB: This code is as first proposed here by Brice Arnould with small changes: https://reviews.apache.org/r/39170/ src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java | 10 -- src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java | 8 +--- src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java | 13 - src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java | 23 +++ src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java | 35 --- 5 files changed, 72 insertions(+), 17 deletions(-) Diffs - src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 043ba7e6858db28001dfb07ea0c2ddf274a1c755 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 1ddc4e1946910de798f7f423dd1b19ed56dece15 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java b85f4abd59ef64264fb089527ad42b9ceee7f8d6 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6efe03fe4841cf1275e2ee0c7cc1b9576540f34e Diff: https://reviews.apache.org/r/41528/diff/ Testing --- Green locally: `./gradlew -Pq build` Thanks, John Sirois
Re: Review Request 39170: Fix NPE on accessing crons set at impossible dates
> On Nov. 15, 2015, 11:34 a.m., John Sirois wrote: > > Brice - this would be nice to land. Are you able to put time towards the > > cleanups suggested by Kevin and style fixes failing the build currently? > > If so - great. If not, I can brush this up and send out a new RB. Brice, I've taken your patch up over here: https://reviews.apache.org/r/41528/ Thanks for this fix! - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39170/#review106566 --- On Oct. 9, 2015, 6:41 a.m., Brice Arnould wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39170/ > --- > > (Updated Oct. 9, 2015, 6:41 a.m.) > > > Review request for Aurora and Kevin Sweeney. > > > Bugs: AURORA-1385 > https://issues.apache.org/jira/browse/AURORA-1385 > > > Repository: aurora > > > Description > --- > > This is for AURORA-1385 > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java > 043ba7e6858db28001dfb07ea0c2ddf274a1c755 > > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java > 1ddc4e1946910de798f7f423dd1b19ed56dece15 > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > 13b5c222a0d7b9dd347990e6c09aac09ee566315 > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > 64cbd5b58d9254bb741e20e47165732e52569f70 > > Diff: https://reviews.apache.org/r/39170/diff/ > > > Testing > --- > > Added a specific unit test > > > Thanks, > > Brice Arnould > >
Re: Review Request 41528: Fixup `getJobSummary` for cron jobs with invalid next run dates.
> On Dec. 17, 2015, 2:17 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java, line 29 > > <https://reviews.apache.org/r/41528/diff/2/?file=1170275#file1170275line29> > > > > nit: is enough to split paragraphs. Thanks - fixed. I could have sworn I got -Xdoclint:all errors for this style recently over in pants land - but I just experimented and definitely am remembering wrong. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41528/#review111058 --- On Dec. 17, 2015, 2:27 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41528/ > --- > > (Updated Dec. 17, 2015, 2:27 p.m.) > > > Review request for Aurora, Stephan Erb, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-1385 > https://issues.apache.org/jira/browse/AURORA-1385 > > > Repository: aurora > > > Description > --- > > Previously, the fact `null` could be returned by Quartz when > calculating the next run was not taken into account. Now The > CronPredictor interface makes this possibility manifest with an > Optional result. > > Code that uses the CronPredictor is adjusted and tests are added. > > NB: This code is as first proposed here by Brice Arnould with small > changes: https://reviews.apache.org/r/39170/ > > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java > | 10 -- > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java > | 8 +--- > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > | 13 - > > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java > | 23 +++ > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > | 35 --- > 5 files changed, 72 insertions(+), 17 deletions(-) > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java > 043ba7e6858db28001dfb07ea0c2ddf274a1c755 > > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java > 1ddc4e1946910de798f7f423dd1b19ed56dece15 > src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java > c0e8a201400338a8cb6bc24b2c21d0abb0d01e41 > > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java > b85f4abd59ef64264fb089527ad42b9ceee7f8d6 > > src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java > 6efe03fe4841cf1275e2ee0c7cc1b9576540f34e > > Diff: https://reviews.apache.org/r/41528/diff/ > > > Testing > --- > > Green locally: `./gradlew -Pq build` > > > Thanks, > > John Sirois > >
Review Request 41893: Fixup missing commons compile dep.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41893/ --- Review request for Aurora and Zameer Manji. Repository: aurora Description --- I get compile errors when working on src/main/java/org/apache/aurora/common/zookeeper/testing. I'm not sure why CI and others do not. build.gradle | 5 + 1 file changed, 5 insertions(+) Diffs - build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 Diff: https://reviews.apache.org/r/41893/diff/ Testing --- Not green locally due to ZK issues from https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7, but the CI script gets further on my machine with the junit dep added Thanks, John Sirois
Re: Review Request 41897: Upgrade to the latest zk point release.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41897/ --- (Updated Jan. 4, 2016, 4:44 p.m.) Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner. Changes --- Fixup NEWS to reflect upgraded upgrade. NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Repository: aurora Description --- This is enabled by https://reviews.apache.org/r/41895/ which isolated the kerberos configuration. build.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Diffs (updated) - NEWS c0c454d080023a2e2796022f957a4d47bdb87b41 build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 Diff: https://reviews.apache.org/r/41897/diff/ Testing --- Both `./build-support/jenkins/build.sh` and `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran green locally. Thanks, John Sirois
Review Request 41895: Fix Kerberos5ShiroRealmModule: use dedicated jaas config.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41895/ --- Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- This fix to Kerberos initialization that moves away from setting a system property to instead use a ConfigFile object directly. This prevents using the default LoginContext internals and entering into races with other components (notably zookeeper). src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java 09d8db4cdaa6e9320bc2e8bb455adf16c4ec64f9 Diff: https://reviews.apache.org/r/41895/diff/ Testing --- I now have `./build-support/jenkins/build.sh` and `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` green locally. Prior to this last fix (see depends-on chain for 2 others), got: ``` org.apache.aurora.scheduler.app.SchedulerIT > testLaunch FAILED java.io.IOException: Could not find a 'Server' entry in this configuration: Server cannot start. at org.apache.zookeeper.server.auth.SaslServerCallbackHandler.(SaslServerCallbackHandler.java:53) at org.apache.zookeeper.server.NIOServerCnxnFactory.configure(NIOServerCnxnFactory.java:96) at org.apache.aurora.common.zookeeper.testing.ZooKeeperTestServer.startNetwork(ZooKeeperTestServer.java:81) at org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest.setUp(BaseZooKeeperTest.java:64) ... ``` Thanks, John Sirois
Re: Review Request 41897: Upgrade to the latest zk point release.
> On Jan. 4, 2016, 4:17 p.m., Stephan Erb wrote: > > News file needs updating too :-) Good call - change coming as well as an updated reviews list. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41897/#review112679 --- On Jan. 4, 2016, 4:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41897/ > --- > > (Updated Jan. 4, 2016, 4:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > This is enabled by https://reviews.apache.org/r/41895/ which > isolated the kerberos configuration. > > build.gradle | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > > Diffs > - > > build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 > > Diff: https://reviews.apache.org/r/41897/diff/ > > > Testing > --- > > Both `./build-support/jenkins/build.sh` and > `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran > green locally. > > > Thanks, > > John Sirois > >
Review Request 41899: Upgrade to pants 0.0.66.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41899/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Repository: aurora Description --- The changelog can be read here: http://pantsbuild.github.io/changelog.html Of note for aurora is the ability to kill BUILD.tools. Additionally, add the pants generated .pids/ dir to .gitignore and normalize all directory ignore to omit the redundant *. .gitignore | 29 +++-- BUILD.tools | 18 -- pants.ini | 2 +- 3 files changed, 16 insertions(+), 33 deletions(-) Diffs - .gitignore 12eff83938329f12ecd0a63043d8f34a608b7e94 BUILD.tools 2e237c43c47fda0771106cbf5a8cccb0b8147710 pants.ini 579d86cd2e02ea3e1a7add9cdd8291a6dc9669ec Diff: https://reviews.apache.org/r/41899/diff/ Testing --- Locally green: ``` ./build-support/jenkins/build.sh && ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, John Sirois
Re: Review Request 41893: Fixup missing commons compile dep.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41893/#review112649 --- Showing my work: ``` $ git grep org.junit | grep src/main/java commons/src/main/java/org/apache/aurora/common/testing/TearDownTestCase.java:import org.junit.After; commons/src/main/java/org/apache/aurora/common/testing/easymock/EasyMockTest.java:import org.junit.Before; commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import org.junit.Before; commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import org.junit.Rule; commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperTest.java:import org.junit.rules.TemporaryFolder; ``` - John Sirois On Jan. 4, 2016, 2:16 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41893/ > --- > > (Updated Jan. 4, 2016, 2:16 p.m.) > > > Review request for Aurora and Zameer Manji. > > > Repository: aurora > > > Description > --- > > I get compile errors when working on > src/main/java/org/apache/aurora/common/zookeeper/testing. > I'm not sure why CI and others do not. > > build.gradle | 5 + > 1 file changed, 5 insertions(+) > > > Diffs > - > > build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 > > Diff: https://reviews.apache.org/r/41893/diff/ > > > Testing > --- > > Not green locally due to ZK issues from > https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7, > but the CI script gets further on my machine with the junit > dep added > > > Thanks, > > John Sirois > >
Re: Review Request 41897: Upgrade to the latest zk point release.
> On Jan. 4, 2016, 4:17 p.m., Stephan Erb wrote: > > News file needs updating too :-) > > John Sirois wrote: > Good call - change coming as well as an updated reviews list. Fixed. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41897/#review112679 --- On Jan. 4, 2016, 4:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41897/ > --- > > (Updated Jan. 4, 2016, 4:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > This is enabled by https://reviews.apache.org/r/41895/ which > isolated the kerberos configuration. > > build.gradle | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > > Diffs > - > > build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75 > > Diff: https://reviews.apache.org/r/41897/diff/ > > > Testing > --- > > Both `./build-support/jenkins/build.sh` and > `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran > green locally. > > > Thanks, > > John Sirois > >
Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41894/ --- (Updated Jan. 4, 2016, 4:04 p.m.) Review request for Aurora and Bill Farner. Summary (updated) - Fixup ZooKeeperTestServer restartNetwork. Repository: aurora Description --- With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to `NIOServerCnxnFactory` came a change in the server internals. Calling `connectionFactory.startup(zooKeeperServer)` now eventually calls `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`. This last is a problem since `SessionTrackerImpl` is a `Thread` and so throws `IllegalThreadStateException`. commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java | 28 1 file changed, 16 insertions(+), 12 deletions(-) Diffs - commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 54b0a09355c1467362c2ad788e217dd5f52de522 Diff: https://reviews.apache.org/r/41894/diff/ Testing --- Not green locally due to ZK/krb interaction issues from https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7, but the CI script gets further on my machine with this fix. Thanks, John Sirois
Re: Review Request 41897: Upgrade to the latest zk point release.
> On Jan. 4, 2016, 4:20 p.m., Aurora ReviewBot wrote:
> > Master (8706a78) is red with this patch.
> > ./build-support/jenkins/build.sh
> >
> >
> > src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_standard_transitions
> > [32mPASSED[0m
> >
> > src/test/python/apache/thermos/observer/test_detector.py::test_observer_task_detector_nonstandard_transitions
> > [32mPASSED[0m
> >
> > src/test/python/apache/thermos/observer/test_task_observer.py::TaskObserverTest::test_run_loop
> > [31mFAILED[0m
> >
> > FAILURES
> > _ TaskObserverTest.test_run_loop _
> >
> > self = > testMethod=test_run_loop>
> >
> > [1mdef test_run_loop(self):[0m
> > [1m """Test observer run loop."""[0m
> > [1m mock_task_detector =
> > create_autospec(spec=ObserverTaskDetector)[0m
> > [1m with patch([0m
> > [1m
> > "apache.thermos.observer.task_observer.ObserverTaskDetector",[0m
> > [1m return_value=mock_task_detector) as
> > mock_detector:[0m
> > [1mwith patch('threading._Event.wait') as
> > mock_wait:[0m
> > [1m[0m
> > [1m run_count = 3[0m
> > [1m interval = 15[0m
> > [1m observer = TaskObserver(mock_detector,
> > interval=Amount(interval, Time.SECONDS))[0m
> > [1m observer.start()[0m
> > [1m while len(mock_wait.mock_calls) <
> > run_count:[0m
> > [1mpass[0m
> > [1m[0m
> > [1m observer.stop()[0m
> > [1m[0m
> > [1m> assert
> > len(mock_task_detector.mock_calls) >= run_count[0m
> > [1m[31mE AssertionError: assert 1 >= 3[0m
> > [1m[31mE + where 1 =
> > len([call.refresh()])[0m
> > [1m[31mE +where [call.refresh()] =
> > .mock_calls[0m
> >
> >
> > src/test/python/apache/thermos/observer/test_task_observer.py:42:
> > AssertionError
> > generated xml file:
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.thermos.observer.observer.xml
> >
> > [1m[31m=== 1 failed, 3 passed in 0.25 seconds
> > ===[0m
> >
> > FAILURE
> >
> >
> > 23:20:37 04:05 [complete][31m
> >FAILURE[0m
> >
> >
> > I will refresh this build result if you post a review containing
> > "@ReviewBot retry"
>
> John Sirois wrote:
> I found no jira issue but will file shortly: Anyone recall if this is
> known flaky?
Filed https://issues.apache.org/jira/browse/AURORA-1570
- John
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41897/#review112683
---
On Jan. 4, 2016, 4:44 p.m., John Sirois wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41897/
> ---
>
> (Updated Jan. 4, 2016, 4:44 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> ---
>
> This is enabled by https://reviews.apache.org/r/41895/ which
> isolated the kerberos configuration.
>
> build.gradle | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>
> Diffs
> -
>
> NEWS c0c454d080023a2e2796022f957a4d47bdb87b41
> build.gradle c1bbb08305b446c2d8aec8d1bf8c6f2299a9db75
>
> Diff: https://reviews.apache.org/r/41897/diff/
>
>
> Testing
> ---
>
> Both `./build-support/jenkins/build.sh` and
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` ran
> green locally.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 41894: Fixup ZooKeeperTestServer restartNetwork.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41894/#review112711 --- "Fixup missing commons compile dep." < not the right RB. - John Sirois On Jan. 4, 2016, 4:04 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41894/ > --- > > (Updated Jan. 4, 2016, 4:04 p.m.) > > > Review request for Aurora and Bill Farner. > > > Repository: aurora > > > Description > --- > > With the upgrade to 3.4.2 and the switch from `NIOServerCnxn.Factory` to > `NIOServerCnxnFactory` came a change in the server internals. Calling > `connectionFactory.startup(zooKeeperServer)` now eventually calls > `ZooKeeperServer.startup` which calls `SessionTrackerImpl.start`. This > last is a problem since `SessionTrackerImpl` is a `Thread` and so throws > `IllegalThreadStateException`. > > > commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java > | 28 > 1 file changed, 16 insertions(+), 12 deletions(-) > > > Diffs > - > > > commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java > 54b0a09355c1467362c2ad788e217dd5f52de522 > > Diff: https://reviews.apache.org/r/41894/diff/ > > > Testing > --- > > Not green locally due to ZK/krb interaction issues from > https://git1-us-west.apache.org/repos/asf/aurora/repo?p=aurora.git;a=commit;h=8706a781968912c68688284d9d3813d34ce45bf7, > but the CI script gets further on my machine with this fix. > > > Thanks, > > John Sirois > >
Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/thermos/observer/task_observer.py, line 72 > > <https://reviews.apache.org/r/41915/diff/1/?file=1181509#file1181509line72> > > > > We usually put this default initialization directly into the arg list, > > e.g:'stop_event=threading.Event()'. > > John Sirois wrote: > I'm leery of that for mutable objects like an Event. Surprising things > happen if/when the containing object gets constructed a 2nd time and the > single default Event has been mutated! Maxim - your test originally so your call. For a refresh - see here: https://reviews.apache.org/r/35527/ - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/#review112750 --- On Jan. 4, 2016, 8:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41915/ > --- > > (Updated Jan. 4, 2016, 8:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-1570 > https://issues.apache.org/jira/browse/AURORA-1570 > > > Repository: aurora > > > Description > --- > > Previously, a mock threading.Event was waited on in one thread > and the count of waits was read in another thread. Most thread > memory models do not guaranty reads are fresh in this scenario > unless there is a memory barrier of some sort forcing per-cpu > caches to be flushed. > > This change uses the underlying threading.Event as the memory > barrier instead of mocking it and just wraps the event to record > calls manually. > > src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- > src/test/python/apache/thermos/observer/test_task_observer.py | 36 > > 2 files changed, 27 insertions(+), 14 deletions(-) > > > Diffs > - > > src/main/python/apache/thermos/observer/task_observer.py > 1485de8faef52716f11b82a3556064de26c67427 > src/test/python/apache/thermos/observer/test_task_observer.py > ace15c5305e75fac3a82971f4d71b92bcb37bafc > > Diff: https://reviews.apache.org/r/41915/diff/ > > > Testing > --- > > Before this change I got a failure between 1/5 and 1/10th of the > time via: > ``` > while true > do > ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest > done > ``` > > After the change I cannot trigger the failure. > > > Thanks, > > John Sirois > >
Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-1570 https://issues.apache.org/jira/browse/AURORA-1570 Repository: aurora Description --- Previously, a mock threading.Event was waited on in one thread and the count of waits was read in another thread. Most thread memory models do not guaranty reads a fresh in this scenario unless there is a memory barrier of some sort forcing per-cpu caches to be flushed. This change uses the underlying threading.Event as the memory barrier instead of mocking it and just wraps the event to record calls manually. src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- src/test/python/apache/thermos/observer/test_task_observer.py | 36 2 files changed, 27 insertions(+), 14 deletions(-) Diffs - src/main/python/apache/thermos/observer/task_observer.py 1485de8faef52716f11b82a3556064de26c67427 src/test/python/apache/thermos/observer/test_task_observer.py ace15c5305e75fac3a82971f4d71b92bcb37bafc Diff: https://reviews.apache.org/r/41915/diff/ Testing --- Before this change I got a failure between 1/5 and 1/10th of the time via: ``` while true do ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest done ``` After the change I cannot trigger the failure. Thanks, John Sirois
Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/thermos/observer/task_observer.py, line 72 > > <https://reviews.apache.org/r/41915/diff/1/?file=1181509#file1181509line72> > > > > We usually put this default initialization directly into the arg list, > > e.g:'stop_event=threading.Event()'. > > John Sirois wrote: > I'm leery of that for mutable objects like an Event. Surprising things > happen if/when the containing object gets constructed a 2nd time and the > single default Event has been mutated! > > John Sirois wrote: > Maxim - your test originally so your call. For a refresh - see here: > https://reviews.apache.org/r/35527/ Disregard comment immediately above - now moved up under Bill's comment further above where it belongs. - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/#review112750 --- On Jan. 4, 2016, 8:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41915/ > --- > > (Updated Jan. 4, 2016, 8:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-1570 > https://issues.apache.org/jira/browse/AURORA-1570 > > > Repository: aurora > > > Description > --- > > Previously, a mock threading.Event was waited on in one thread > and the count of waits was read in another thread. Most thread > memory models do not guaranty reads are fresh in this scenario > unless there is a memory barrier of some sort forcing per-cpu > caches to be flushed. > > This change uses the underlying threading.Event as the memory > barrier instead of mocking it and just wraps the event to record > calls manually. > > src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- > src/test/python/apache/thermos/observer/test_task_observer.py | 36 > > 2 files changed, 27 insertions(+), 14 deletions(-) > > > Diffs > - > > src/main/python/apache/thermos/observer/task_observer.py > 1485de8faef52716f11b82a3556064de26c67427 > src/test/python/apache/thermos/observer/test_task_observer.py > ace15c5305e75fac3a82971f4d71b92bcb37bafc > > Diff: https://reviews.apache.org/r/41915/diff/ > > > Testing > --- > > Before this change I got a failure between 1/5 and 1/10th of the > time via: > ``` > while true > do > ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest > done > ``` > > After the change I cannot trigger the failure. > > > Thanks, > > John Sirois > >
Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
> On Jan. 4, 2016, 8:05 p.m., John Sirois wrote: > > Another answer could be to delete this test altogether. It looks like it > > only really tests the proper converson from Time Amounts to fractional > > second waits. > > Bill Farner wrote: > I'm tempted to take this route as well. It's not clear to me what this > test accomplishes, and it has the downside of high coupling to the > implementation being tested. Maxim - your test originally so your call. For a refresh - see here: https://reviews.apache.org/r/35527/ - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/#review112748 --- On Jan. 4, 2016, 8:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41915/ > --- > > (Updated Jan. 4, 2016, 8:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-1570 > https://issues.apache.org/jira/browse/AURORA-1570 > > > Repository: aurora > > > Description > --- > > Previously, a mock threading.Event was waited on in one thread > and the count of waits was read in another thread. Most thread > memory models do not guaranty reads are fresh in this scenario > unless there is a memory barrier of some sort forcing per-cpu > caches to be flushed. > > This change uses the underlying threading.Event as the memory > barrier instead of mocking it and just wraps the event to record > calls manually. > > src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- > src/test/python/apache/thermos/observer/test_task_observer.py | 36 > > 2 files changed, 27 insertions(+), 14 deletions(-) > > > Diffs > - > > src/main/python/apache/thermos/observer/task_observer.py > 1485de8faef52716f11b82a3556064de26c67427 > src/test/python/apache/thermos/observer/test_task_observer.py > ace15c5305e75fac3a82971f4d71b92bcb37bafc > > Diff: https://reviews.apache.org/r/41915/diff/ > > > Testing > --- > > Before this change I got a failure between 1/5 and 1/10th of the > time via: > ``` > while true > do > ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest > done > ``` > > After the change I cannot trigger the failure. > > > Thanks, > > John Sirois > >
Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/ --- (Updated Jan. 4, 2016, 8:02 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-1570 https://issues.apache.org/jira/browse/AURORA-1570 Repository: aurora Description (updated) --- Previously, a mock threading.Event was waited on in one thread and the count of waits was read in another thread. Most thread memory models do not guaranty reads are fresh in this scenario unless there is a memory barrier of some sort forcing per-cpu caches to be flushed. This change uses the underlying threading.Event as the memory barrier instead of mocking it and just wraps the event to record calls manually. src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- src/test/python/apache/thermos/observer/test_task_observer.py | 36 2 files changed, 27 insertions(+), 14 deletions(-) Diffs - src/main/python/apache/thermos/observer/task_observer.py 1485de8faef52716f11b82a3556064de26c67427 src/test/python/apache/thermos/observer/test_task_observer.py ace15c5305e75fac3a82971f4d71b92bcb37bafc Diff: https://reviews.apache.org/r/41915/diff/ Testing --- Before this change I got a failure between 1/5 and 1/10th of the time via: ``` while true do ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest done ``` After the change I cannot trigger the failure. Thanks, John Sirois
Re: Review Request 41915: Fixup TaskObserverTest to respect thread memory models.
> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/thermos/observer/task_observer.py, line 72 > > <https://reviews.apache.org/r/41915/diff/1/?file=1181509#file1181509line72> > > > > We usually put this default initialization directly into the arg list, > > e.g:'stop_event=threading.Event()'. I'm leery of that for mutable objects like an Event. Surprising things happen if/when the containing object gets constructed a 2nd time and the single default Event has been mutated! - John --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/#review112750 ------- On Jan. 4, 2016, 8:02 p.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41915/ > --- > > (Updated Jan. 4, 2016, 8:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-1570 > https://issues.apache.org/jira/browse/AURORA-1570 > > > Repository: aurora > > > Description > --- > > Previously, a mock threading.Event was waited on in one thread > and the count of waits was read in another thread. Most thread > memory models do not guaranty reads are fresh in this scenario > unless there is a memory barrier of some sort forcing per-cpu > caches to be flushed. > > This change uses the underlying threading.Event as the memory > barrier instead of mocking it and just wraps the event to record > calls manually. > > src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- > src/test/python/apache/thermos/observer/test_task_observer.py | 36 > > 2 files changed, 27 insertions(+), 14 deletions(-) > > > Diffs > - > > src/main/python/apache/thermos/observer/task_observer.py > 1485de8faef52716f11b82a3556064de26c67427 > src/test/python/apache/thermos/observer/test_task_observer.py > ace15c5305e75fac3a82971f4d71b92bcb37bafc > > Diff: https://reviews.apache.org/r/41915/diff/ > > > Testing > --- > > Before this change I got a failure between 1/5 and 1/10th of the > time via: > ``` > while true > do > ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest > done > ``` > > After the change I cannot trigger the failure. > > > Thanks, > > John Sirois > >
