> On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote: > >
Thanks for the review! > On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote: > > builder/deb/debian-jessie/build.sh, line 28 > > <https://reviews.apache.org/r/52437/diff/4/?file=1621611#file1621611line28> > > > > That indentation seems unnecessary. Fixed! > On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote: > > builder/deb/debian-jessie/build.sh, lines 30-31 > > <https://reviews.apache.org/r/52437/diff/4/?file=1621611#file1621611line30> > > > > It is more robust if we inject the whole section here (including the > > section header). Yep, that makes sense. I'll go ahead and fix that. > On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote: > > specs/debian/aurora-executor.thermos.service, lines 22-23 > > <https://reviews.apache.org/r/52437/diff/4/?file=1621617#file1621617line22> > > > > Can the corresponding init scripts be dropped? We can definitely drop them but we'd lose (unofficial) compatibility for those who chose to init sysv init system (debian wheezy). > On Feb. 6, 2017, 1:01 p.m., Stephan Erb wrote: > > specs/debian/aurora-executor.thermos.service, lines 19-21 > > <https://reviews.apache.org/r/52437/diff/4/?file=1621617#file1621617line19> > > > > Why have you specified both `Environment` and `EnvironmentFile`. I > > suppose the latter has precedence? Great catch, I'd forgotten I'd done this. Yup, EnvironmentFile has preedence. In this case EnvironmentFile also has a negative sign after the equal sign to ignore errors (e.g. file doesn't exist). I was experimenting with having a fallback option if the config file wasn't found, but since the deb package places one there, we can assume it'll be there. I'll get rid of lines 19 and 20. - Renan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52437/#review164402 ----------------------------------------------------------- On Feb. 1, 2017, 12:44 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52437/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2017, 12:44 p.m.) > > > Review request for Aurora, Dmitriy Shirchenko and Zameer Manji. > > > Bugs: AURORA-1872 > https://issues.apache.org/jira/browse/AURORA-1872 > > > Repository: aurora-packaging > > > Description > ------- > > Added builder and test environment for Xenial as well as updated instructions > on how to test it. > > Added distribution to release-candidate script. > > > Diffs > ----- > > build-support/release/release-candidate > b3ebe916102d0a6ef2c1997b6c4110008eafff1a > builder/deb/debian-jessie/Dockerfile > 54cb2e072a0d465840899e00e541cec34130993d > builder/deb/debian-jessie/build.sh e0aeaf52a29483e65172f29792638dfbca100079 > builder/deb/ubuntu-trusty/Dockerfile > 802049b6b2a53666c153525f2fb97bfbf0d4bab4 > builder/deb/ubuntu-trusty/build.sh e0aeaf52a29483e65172f29792638dfbca100079 > builder/deb/ubuntu-xenial/Dockerfile PRE-CREATION > builder/deb/ubuntu-xenial/build.sh PRE-CREATION > specs/debian/aurora-executor.thermos.default > 82ab0c8ab585acf253f8fecc0ed88a3e19511ffb > specs/debian/aurora-executor.thermos.service > da351164298bd2b5a1802a945211647c99193ae6 > specs/debian/aurora-pants.ini e4f2c54df45f746ffb2a4e3f786fc4105323bc0b > specs/debian/aurora-scheduler.service > ae33d26ac10518985eacea6bf015f81a02c53f66 > specs/debian/aurora-scheduler.startup.sh PRE-CREATION > specs/debian/rules 4effc3ecf3208f82986d15f6000d18abc11652b1 > test/deb/ubuntu-xenial/README.md PRE-CREATION > test/deb/ubuntu-xenial/Vagrantfile PRE-CREATION > test/deb/ubuntu-xenial/provision.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/52437/diff/ > > > Testing > ------- > > Created artifacts using the build-artifacts script. > > Brought a vagrant image up, installed all deb files created by the artifacts > script, started both aurora-scheduler and thermos services, and launched a > sample job. > > Made sure all debian systems were unaffected by the changes by bringing a > Vagrant image up for each distribution and installing the packages. > > > Thanks, > > Renan DelValle > >
