Re: Review Request 37764: Update packages for post 0.9.0 changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37764/ --- (Updated Aug. 25, 2015, 10:38 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- Update packages for post 0.9.0 changes. Diffs (updated) - builder/deb/ubuntu-trusty/Dockerfile 73f150b987a1053ca01e80d6bf2d3658ec87a569 specs/debian/control 3ece2023aab16e9dd90e5ad5a84d172c172854d5 specs/debian/rules ef35a04a443ad5ac84e84ea1e92bce3b898d9324 specs/rpm/aurora.spec e6fca4e4f3e77f1394ec3411a3ea680c4771cf82 Diff: https://reviews.apache.org/r/37764/diff/ Testing --- Deb building works, next step is to begin executing this in jenkins. Thanks, Bill Farner
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96377 --- Ship it! Ship It! - Zameer Manji On Aug. 25, 2015, 8:01 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 8:01 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96362 --- Ship it! Ship It! - Maxim Khutornenko On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Review Request 37764: Update packages for post 0.9.0 changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37764/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- Update packages for post 0.9.0 changes. Diffs - builder/deb/ubuntu-trusty/Dockerfile 73f150b987a1053ca01e80d6bf2d3658ec87a569 specs/debian/control 3ece2023aab16e9dd90e5ad5a84d172c172854d5 specs/debian/rules ef35a04a443ad5ac84e84ea1e92bce3b898d9324 specs/rpm/aurora.spec e6fca4e4f3e77f1394ec3411a3ea680c4771cf82 Diff: https://reviews.apache.org/r/37764/diff/ Testing --- Deb building works, next step is to begin executing this in jenkins. Thanks, Bill Farner
Re: Review Request 37764: Update packages for post 0.9.0 changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37764/#review96374 --- Ship it! Ship It! - Maxim Khutornenko On Aug. 25, 2015, 5:38 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37764/ --- (Updated Aug. 25, 2015, 5:38 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- Update packages for post 0.9.0 changes. Diffs - builder/deb/ubuntu-trusty/Dockerfile 73f150b987a1053ca01e80d6bf2d3658ec87a569 specs/debian/control 3ece2023aab16e9dd90e5ad5a84d172c172854d5 specs/debian/rules ef35a04a443ad5ac84e84ea1e92bce3b898d9324 specs/rpm/aurora.spec e6fca4e4f3e77f1394ec3411a3ea680c4771cf82 Diff: https://reviews.apache.org/r/37764/diff/ Testing --- Deb building works, next step is to begin executing this in jenkins. Thanks, Bill Farner
Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37742: Remove use of host attributes from e2e tests.
On Aug. 25, 2015, 3:31 a.m., Maxim Khutornenko wrote:
src/test/sh/org/apache/aurora/e2e/http/http_example.aurora, lines 45-48
https://reviews.apache.org/r/37742/diff/1/?file=1049818#file1049818line45
I am a bit concerned we will no longer have any constraint coverage in
our e2e tests. Can you clarify why post deploy RPM env altering is not an
option here?
Bill Farner wrote:
It's certainly an option, but testing against vanilla setups lowers the
barriers pretty considerably since things are likely to be
organized/configured differently on different platforms. I'm not firm on
this change, but i do find the point of constraint coverage a weak one
without an explicit test case for it (rather than the current repetition
which is really a relic of old behavior).
I see your point about lowering testing barriers. At the same time, it feels
like having an artificial restraint on modifying slave config prevents us from
adding constraint tests in either approach (e2e or RPM). I'd rather have an
option to address lack of coverage later if needed.
Splitting RPM and e2e test configs seems like a better way forward to me. This
will let us keep RPM tests simple and lean (i.e. just run a subset of e2e if
needed) while not precluding custom mods needed to test specific features. One
example: oversubscription tests will require adding somthing like this into the
slave config, which may or may not be of value to RPM testing:
```
--resource_estimator=org_apache_mesos_FixedResourceEstimator
--modules={libraries:[{file:/usr/local/lib64/libfixed_resource_estimator.so,modules:[{name:org_apache_mesos_FixedResourceEstimator,parameters:[{key:resources,value:cpus:8}]}]}]}
```
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37742/#review96286
---
On Aug. 25, 2015, 1:11 a.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37742/
---
(Updated Aug. 25, 2015, 1:11 a.m.)
Review request for Aurora and Maxim Khutornenko.
Repository: aurora
Description
---
These are no longer needed now that the scheduler isn't injecting these
settings by default for jobs. It's beneficial for them to be removed, as it
will make it easier to point the e2e test at an arbitrary environment to
exercise it. I would like to use that approach to test RPMs/debs.
Diffs
-
examples/vagrant/upstart/mesos-slave.conf
2b6a60673fc0a7ea3b73471701cd5d3efd6ce639
src/test/sh/org/apache/aurora/e2e/http/http_example.aurora
c1a10d8ea60be6aa56e4517fb34288d7d5ae1480
src/test/sh/org/apache/aurora/e2e/http/http_example_docker.aurora
870b3e68035fdf86253cf9b92b606645134b3369
src/test/sh/org/apache/aurora/e2e/http/http_example_docker_updated.aurora
e55aad3a58d4e3c19332e06b70771f51f07aa9b7
src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora
423dd4d4e8b03c2f852e25acd9340bd6288b7d24
Diff: https://reviews.apache.org/r/37742/diff/
Testing
---
e2e tests pass
Thanks,
Bill Farner
Re: Review Request 37772: Fix RPM building.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 25, 2015, 9:38 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description (updated) --- Fix references when copying package sources files. Diffs (updated) - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37772: Fix RPM building.
On Aug. 25, 2015, 4:12 p.m., Kevin Sweeney wrote: specs/rpm/Makefile, lines 37-38 https://reviews.apache.org/r/37772/diff/1/?file=1052463#file1052463line37 This is weird as a user will never be able to upgrade from a release version to a nightly. Maxim Khutornenko wrote: Isn't version global across RPM builds? This was a misunderstanding of how epochs are used for version identification. I will back out that part of the patch. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/#review96443 --- On Aug. 25, 2015, 2:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 25, 2015, 2:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- This addresses several things needed to get RPM builds working. The major change is the specification of an RPM Epoch, which will allow the desired version ordering for nightly vs released versions. If you would like some light reading on this topic, you can find it here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37772: Fix RPM building.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/#review96494 --- Ship it! specs/rpm/aurora.spec (lines 62 - 72) https://reviews.apache.org/r/37772/#comment151883 nit: could be easier to follow if source definitions matched the below install sequence. - Maxim Khutornenko On Aug. 26, 2015, 4:38 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 26, 2015, 4:38 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- Fix references when copying package sources files. Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Review Request 37788: Deb: Clean up changelog generation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37788/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora-packaging Description --- This sets the maintainer fields, and ensures we mark the changelog entry as released. Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 Diff: https://reviews.apache.org/r/37788/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96399 --- Master (86a547b) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? - David McLaughlin On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37719: Revocable: schema changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37719/ --- (Updated Aug. 25, 2015, 10:28 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Removing hardcoded tier mapping. Will rather spend time fully implementing TierManager next to properly translate tier definitions. Bugs: AURORA-1414 https://issues.apache.org/jira/browse/AURORA-1414 Repository: aurora Description --- Added support for 'tier' in schema and thrift. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 Diff: https://reviews.apache.org/r/37719/diff/ Testing --- ./gradlew -Pq build ./pants test.pytest --no-fast src/test/python:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
On Aug. 25, 2015, 6:42 p.m., David McLaughlin wrote: What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? I don't think we care, there's not much we can do in that case, is there? The only thing I can think of is disabling links for completed updates, but that seems overly broad (some completed updates will link to instances that still exist). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37770: Replace Twitter copyright headers in commons.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37770/#review96420 --- Ship it! Ship It! - Bill Farner On Aug. 25, 2015, 1:16 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37770/ --- (Updated Aug. 25, 2015, 1:16 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1442 https://issues.apache.org/jira/browse/AURORA-1442 Repository: aurora Description --- The IP was donated to the Apache Foundation so the copyright headers need to be updated to reflect their new status. Diffs - build.gradle 505270819e22cac2727f2b8e73070c3288591b61 commons-args/src/main/java/com/twitter/common/args/Arg.java 2f18029037d33024d3ea3d537f8af8aee0afbd07 commons-args/src/main/java/com/twitter/common/args/ArgParser.java 9900f6f0b48eb3099b74fbe3a1f9f85cb80ceafd commons-args/src/main/java/com/twitter/common/args/CmdLine.java 13453d95183c45a4e5ace30202b48ab6cd13977b commons-args/src/main/java/com/twitter/common/args/NoParser.java f58beafb2c072f08cb2f67c0bf235d8ad5dbc3f3 commons-args/src/main/java/com/twitter/common/args/Parser.java 947ef0cdf0da91c06ed4c3a26871d29296c963fc commons-args/src/main/java/com/twitter/common/args/ParserOracle.java f9fccd4ccaceca59822184f5e8289309adb6c4fa commons-args/src/main/java/com/twitter/common/args/Positional.java 92cf660a45a3555304389442ba9148b57b90d78d commons-args/src/main/java/com/twitter/common/args/Verifier.java 763cd69519abc6e5157187d9062dd53ba8f0dde8 commons-args/src/main/java/com/twitter/common/args/VerifierFor.java 39ce525fde18ca68175a421aab8a0d2e55b36adf commons-args/src/main/java/com/twitter/common/args/apt/CmdLineProcessor.java ab1f25553d25cf81900b437481c9719689e53516 commons-args/src/main/java/com/twitter/common/args/apt/Configuration.java 1254fc27b6a32002565b336e17db0b62bc4b2481 commons/src/main/java/com/twitter/common/application/AbstractApplication.java 239a9ef7089531636f6ce9a6c1aed6de9ec02e7c commons/src/main/java/com/twitter/common/application/AppLauncher.java 6b4ccc3fbb211affdbed1c5d46f284a10a184548 commons/src/main/java/com/twitter/common/application/Application.java c3203c08bf0d5b6c72bee75bd505df2d2cff4636 commons/src/main/java/com/twitter/common/application/Lifecycle.java 28a667df5e2182fc4ee0e37f9ab7b51f4990da66 commons/src/main/java/com/twitter/common/application/ShutdownRegistry.java 993d273fa6740775477a5353d6c0bee0a1a65fef commons/src/main/java/com/twitter/common/application/ShutdownStage.java 8c8b2bdcc74d9e776d58dd74bd3b3a05e32430f7 commons/src/main/java/com/twitter/common/application/StartupRegistry.java 0643affe415f55cfca5ae71eb24c72bacdbd847d commons/src/main/java/com/twitter/common/application/StartupStage.java b8e6a521007cdfaacb52b8a0e322ab4b1ba66945 commons/src/main/java/com/twitter/common/application/http/DefaultQuitHandler.java 2b5d0d5121845e383d73da01dcff63b219c1a16a commons/src/main/java/com/twitter/common/application/http/GraphViewer.java 5313c7e0fb5e58a369cf320f51211fb30e359685 commons/src/main/java/com/twitter/common/application/http/HttpAssetConfig.java 494075105376d4ec8e7952875321e3a03507b246 commons/src/main/java/com/twitter/common/application/http/HttpFilterConfig.java 864c621e20070e62e85c75dc3b77370de2043e89 commons/src/main/java/com/twitter/common/application/http/HttpServletConfig.java 00479f06e1dcab18ab8f6f580e89362629c2475a commons/src/main/java/com/twitter/common/application/http/Registration.java b17bd851c746740e340f4ad59cae39cd657255b2 commons/src/main/java/com/twitter/common/application/modules/AppLauncherModule.java 0145e026dc040d8c48e83baebafd929eca032a63 commons/src/main/java/com/twitter/common/application/modules/LifecycleModule.java 49f47800596a2b3938484904700334d7c9d4f9cc commons/src/main/java/com/twitter/common/application/modules/LocalServiceRegistry.java 63f50cb8ca1c8ac91602fb420b1994bfb60aba1d commons/src/main/java/com/twitter/common/application/modules/LogModule.java b019c3ed652a27b144114b0ba4fa754d69ffb872 commons/src/main/java/com/twitter/common/application/modules/StatsExportModule.java 82e4cf05aac6a39c617438b04ec0d618d668491c commons/src/main/java/com/twitter/common/application/modules/StatsModule.java 4262aa70287177d869b39615a7fb0f70f371ad69 commons/src/main/java/com/twitter/common/application/modules/ThriftModule.java f55cafb843846ec67bf87c5e302ef5d9da43ca72 commons/src/main/java/com/twitter/common/args/ArgFilters.java 2b5442b516abf74546a3a84dc99a224d9a708973
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
On Aug. 25, 2015, 6:42 p.m., David McLaughlin wrote: What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? Joshua Cohen wrote: I don't think we care, there's not much we can do in that case, is there? The only thing I can think of is disabling links for completed updates, but that seems overly broad (some completed updates will link to instances that still exist). David McLaughlin wrote: Right. I'm just concerned people click on the link to see what happened on that instance event. I know it's what I would expect, given it's the only link and the first column on the row too. Basically it comes down - the instance page you added is a 'live' view of that instance. It is potentially misleading to include that link on a table that deals exclusively with historical data. Yeah, I understand that, I'm just not sure what the alternative is other than not linking at all (which seems worse to me)? As far as I can see there's no association between an instance in an update and an actual task id (which would let us query to see if the scheduler still has a record of that task existing before displaying the link). That said, even if we *could* conditionally display the link, that might be even more confusing as it would be feasible that only *some* tasks from a historical update have been purged, while others might still remain, leading to a strange inconsistency on the update page where only some instances are links, while others are not. As far as I see it we have three options: 1) Always link to the instance page. 2) Only link to the instance page for active updates. 3) Never link to the instance page. Option 1 seems like the best option in that it provides an easy way to see what happened for an update, and in most likely cases (debugging an active or recently completed update) should provide useful data (though admittedly could prove to be confusing in the cases where the task that was part of an update has already been pruned). - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37776: Setting revocable flag on a TaskInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37776/#review96459 --- Ship it! Master (96a086c) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 25, 2015, 11:05 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37776/ --- (Updated Aug. 25, 2015, 11:05 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- In order to be launched by Mesos a BE task resources must have a `revocable` flag set. The ExecutorInfo revocable flag is not strictly necessary but using the same approach simplifies resource handling. Diffs - src/jmh/java/org/apache/aurora/benchmark/Offers.java 9f3ce1643c583ea4160478c57da9c559beb38c4a src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 src/test/java/org/apache/aurora/scheduler/ResourcesTest.java c48d0968970601b86b2e99cc1e3defaddd24bf28 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 8a1599a0c71bc598bb19c69a36bb380d49387710 Diff: https://reviews.apache.org/r/37776/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
On Aug. 25, 2015, 6:42 p.m., David McLaughlin wrote: What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? Joshua Cohen wrote: I don't think we care, there's not much we can do in that case, is there? The only thing I can think of is disabling links for completed updates, but that seems overly broad (some completed updates will link to instances that still exist). David McLaughlin wrote: Right. I'm just concerned people click on the link to see what happened on that instance event. I know it's what I would expect, given it's the only link and the first column on the row too. Basically it comes down - the instance page you added is a 'live' view of that instance. It is potentially misleading to include that link on a table that deals exclusively with historical data. Joshua Cohen wrote: Yeah, I understand that, I'm just not sure what the alternative is other than not linking at all (which seems worse to me)? As far as I can see there's no association between an instance in an update and an actual task id (which would let us query to see if the scheduler still has a record of that task existing before displaying the link). That said, even if we *could* conditionally display the link, that might be even more confusing as it would be feasible that only *some* tasks from a historical update have been purged, while others might still remain, leading to a strange inconsistency on the update page where only some instances are links, while others are not. As far as I see it we have three options: 1) Always link to the instance page. 2) Only link to the instance page for active updates. 3) Never link to the instance page. Option 1 seems like the best option in that it provides an easy way to see what happened for an update, and in most likely cases (debugging an active or recently completed update) should provide useful data (though admittedly could prove to be confusing in the cases where the task that was part of an update has already been pruned). David McLaughlin wrote: I'm leaning towards (3) but maybe that's because I don't understand where the requirement comes from. The linked ticket suggests the user story is I see an instance event with a 'bad' event status and I want to click straight through to see why.' I see the main use case for this occuring when you have a bad deploy, but if rollbacks are enabled you'd click the event and get taken to the rolled back instance page. This could be frustrating at best and misleading/confusing at worst. Happy to be overruled by a tie-breaking vote here. | but if rollbacks are enabled you'd click the event and get taken to the rolled back instance page. The instance page aggregates task history by instance ID, right? This is a perfect example when having instance history is actually quite useful when debugging a rollback. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37774: Adding TierManager binding into JMH benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37774/#review96439 --- Ship it! Ship It! - Bill Farner On Aug. 25, 2015, 3:16 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37774/ --- (Updated Aug. 25, 2015, 3:16 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Fixing missing TierManager binding. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java e41b299a2490a77d3e177b883c99e1c4fcbb6499 src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 425d27c57b93f4172147501c41b15aeef0819455 Diff: https://reviews.apache.org/r/37774/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 37772: Fix RPM building.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/#review96443 --- specs/rpm/Makefile (lines 37 - 38) https://reviews.apache.org/r/37772/#comment151814 This is weird as a user will never be able to upgrade from a release version to a nightly. - Kevin Sweeney On Aug. 25, 2015, 2:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 25, 2015, 2:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- This addresses several things needed to get RPM builds working. The major change is the specification of an RPM Epoch, which will allow the desired version ordering for nightly vs released versions. If you would like some light reading on this topic, you can find it here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37772: Fix RPM building.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/#review96450 --- specs/rpm/Makefile (line 33) https://reviews.apache.org/r/37772/#comment151837 Shouldn't nightly also use AURORA_EPOCH? It's unlikely but possible to envision multiple invocations of a nightly build interleaved with commits producing unexpected results. In fact, would it make sense to make AURORA_EPOCH global and auto-incrementing (e.g. off of system time)? - Maxim Khutornenko On Aug. 25, 2015, 9:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 25, 2015, 9:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- This addresses several things needed to get RPM builds working. The major change is the specification of an RPM Epoch, which will allow the desired version ordering for nightly vs released versions. If you would like some light reading on this topic, you can find it here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37719: Revocable: schema changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37719/#review96438 --- Ship it! Ship It! - Bill Farner On Aug. 25, 2015, 3:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37719/ --- (Updated Aug. 25, 2015, 3:28 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1414 https://issues.apache.org/jira/browse/AURORA-1414 Repository: aurora Description --- Added support for 'tier' in schema and thrift. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 Diff: https://reviews.apache.org/r/37719/diff/ Testing --- ./gradlew -Pq build ./pants test.pytest --no-fast src/test/python:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Review Request 37776: Setting revocable flag on a TaskInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37776/ --- Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- In order to be launched by Mesos a BE task resources must have a `revocable` flag set. The ExecutorInfo revocable flag is not strictly necessary but using the same approach simplifies resource handling. Diffs - src/jmh/java/org/apache/aurora/benchmark/Offers.java 9f3ce1643c583ea4160478c57da9c559beb38c4a src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 src/test/java/org/apache/aurora/scheduler/ResourcesTest.java c48d0968970601b86b2e99cc1e3defaddd24bf28 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 8a1599a0c71bc598bb19c69a36bb380d49387710 Diff: https://reviews.apache.org/r/37776/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 37772: Fix RPM building.
On Aug. 25, 2015, 11:12 p.m., Kevin Sweeney wrote: specs/rpm/Makefile, lines 37-38 https://reviews.apache.org/r/37772/diff/1/?file=1052463#file1052463line37 This is weird as a user will never be able to upgrade from a release version to a nightly. Isn't version global across RPM builds? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/#review96443 --- On Aug. 25, 2015, 9:13 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- (Updated Aug. 25, 2015, 9:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- This addresses several things needed to get RPM builds working. The major change is the specification of an RPM Epoch, which will allow the desired version ordering for nightly vs released versions. If you would like some light reading on this topic, you can find it here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
On Aug. 25, 2015, 6:42 p.m., David McLaughlin wrote: What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? Joshua Cohen wrote: I don't think we care, there's not much we can do in that case, is there? The only thing I can think of is disabling links for completed updates, but that seems overly broad (some completed updates will link to instances that still exist). David McLaughlin wrote: Right. I'm just concerned people click on the link to see what happened on that instance event. I know it's what I would expect, given it's the only link and the first column on the row too. Basically it comes down - the instance page you added is a 'live' view of that instance. It is potentially misleading to include that link on a table that deals exclusively with historical data. Joshua Cohen wrote: Yeah, I understand that, I'm just not sure what the alternative is other than not linking at all (which seems worse to me)? As far as I can see there's no association between an instance in an update and an actual task id (which would let us query to see if the scheduler still has a record of that task existing before displaying the link). That said, even if we *could* conditionally display the link, that might be even more confusing as it would be feasible that only *some* tasks from a historical update have been purged, while others might still remain, leading to a strange inconsistency on the update page where only some instances are links, while others are not. As far as I see it we have three options: 1) Always link to the instance page. 2) Only link to the instance page for active updates. 3) Never link to the instance page. Option 1 seems like the best option in that it provides an easy way to see what happened for an update, and in most likely cases (debugging an active or recently completed update) should provide useful data (though admittedly could prove to be confusing in the cases where the task that was part of an update has already been pruned). I'm leaning towards (3) but maybe that's because I don't understand where the requirement comes from. The linked ticket suggests the user story is I see an instance event with a 'bad' event status and I want to click straight through to see why.' I see the main use case for this occuring when you have a bad deploy, but if rollbacks are enabled you'd click the event and get taken to the rolled back instance page. This could be frustrating at best and misleading/confusing at worst. Happy to be overruled by a tie-breaking vote here. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Review Request 37772: Fix RPM building.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37772/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-851 https://issues.apache.org/jira/browse/AURORA-851 Repository: aurora-packaging Description --- This addresses several things needed to get RPM builds working. The major change is the specification of an RPM Epoch, which will allow the desired version ordering for nightly vs released versions. If you would like some light reading on this topic, you can find it here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html Diffs - builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 specs/rpm/aurora.init.sh specs/rpm/aurora.logrotate specs/rpm/aurora.service specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e specs/rpm/aurora.startup.sh specs/rpm/aurora.sysconfig specs/rpm/clusters.json specs/rpm/thermos-observer.init.sh specs/rpm/thermos-observer.logrotate specs/rpm/thermos-observer.service specs/rpm/thermos-observer.startup.sh specs/rpm/thermos-observer.sysconfig Diff: https://reviews.apache.org/r/37772/diff/ Testing --- Successfully ran ``` ./build-artifact.sh builder/rpm/centos-7 ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25 ``` Thanks, Bill Farner
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
On Aug. 25, 2015, 6:42 p.m., David McLaughlin wrote: What if they click on an old update and the instance page doesn't reflect the change made in this instance event? Do we care? Joshua Cohen wrote: I don't think we care, there's not much we can do in that case, is there? The only thing I can think of is disabling links for completed updates, but that seems overly broad (some completed updates will link to instances that still exist). David McLaughlin wrote: Right. I'm just concerned people click on the link to see what happened on that instance event. I know it's what I would expect, given it's the only link and the first column on the row too. Basically it comes down - the instance page you added is a 'live' view of that instance. It is potentially misleading to include that link on a table that deals exclusively with historical data. Joshua Cohen wrote: Yeah, I understand that, I'm just not sure what the alternative is other than not linking at all (which seems worse to me)? As far as I can see there's no association between an instance in an update and an actual task id (which would let us query to see if the scheduler still has a record of that task existing before displaying the link). That said, even if we *could* conditionally display the link, that might be even more confusing as it would be feasible that only *some* tasks from a historical update have been purged, while others might still remain, leading to a strange inconsistency on the update page where only some instances are links, while others are not. As far as I see it we have three options: 1) Always link to the instance page. 2) Only link to the instance page for active updates. 3) Never link to the instance page. Option 1 seems like the best option in that it provides an easy way to see what happened for an update, and in most likely cases (debugging an active or recently completed update) should provide useful data (though admittedly could prove to be confusing in the cases where the task that was part of an update has already been pruned). David McLaughlin wrote: I'm leaning towards (3) but maybe that's because I don't understand where the requirement comes from. The linked ticket suggests the user story is I see an instance event with a 'bad' event status and I want to click straight through to see why.' I see the main use case for this occuring when you have a bad deploy, but if rollbacks are enabled you'd click the event and get taken to the rolled back instance page. This could be frustrating at best and misleading/confusing at worst. Happy to be overruled by a tie-breaking vote here. Maxim Khutornenko wrote: | but if rollbacks are enabled you'd click the event and get taken to the rolled back instance page. The instance page aggregates task history by instance ID, right? This is a perfect example when having instance history is actually quite useful when debugging a rollback. Exactly. There's no rolled back instance page. There's just the instance page that shows you everything that's happened recently for a particular instance id. So based on your example of a rollback, you'd see the Active task as the rolled back one, and under the list of completed tasks, you'd see the one that failed and (hopefully) more easily be able to determine what went wrong. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96394 --- On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37761: Add a link to the instance page from instance events on the update page.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/#review96479 --- Ship it! Ship It! - David McLaughlin On Aug. 25, 2015, 3:01 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37761/ --- (Updated Aug. 25, 2015, 3:01 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1331 https://issues.apache.org/jira/browse/AURORA-1331 Repository: aurora Description --- Add a link to the instance page from instance events on the update page. Diffs - src/main/resources/scheduler/assets/update.html 3750aab342e326fc34d254dbfce791da0ca05ec6 Diff: https://reviews.apache.org/r/37761/diff/ Testing --- See screenshot. File Attachments Look, the instance numbers are blue, because they're links! https://reviews.apache.org/media/uploaded/files/2015/08/25/b0dc6715-be1a-4a81-992f-caf2efd847a6__Screen_Shot_2015-08-25_at_9.59.50_AM.png Thanks, Joshua Cohen
Re: Review Request 37776: Setting revocable flag on a TaskInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37776/#review96480 --- Ship it! Ship It! - Bill Farner On Aug. 25, 2015, 4:05 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37776/ --- (Updated Aug. 25, 2015, 4:05 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- In order to be launched by Mesos a BE task resources must have a `revocable` flag set. The ExecutorInfo revocable flag is not strictly necessary but using the same approach simplifies resource handling. Diffs - src/jmh/java/org/apache/aurora/benchmark/Offers.java 9f3ce1643c583ea4160478c57da9c559beb38c4a src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 src/test/java/org/apache/aurora/scheduler/ResourcesTest.java c48d0968970601b86b2e99cc1e3defaddd24bf28 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 8a1599a0c71bc598bb19c69a36bb380d49387710 Diff: https://reviews.apache.org/r/37776/diff/ Testing --- Thanks, Maxim Khutornenko
