date:20171010

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187612
---



This patch does not apply cleanly against master (519e3df), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> bb7055ed6826d57cb6b5c5c77e4a27c3bcdd10c7 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 0a2516e4843b8f920700ece70b3cc816d2acecf0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 835f1604c0c5d913a87d570ee01d053bbbf92ecb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 9d2164f4f2d18e4595d3039d64cedacb7df00ae2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> d145259653dd4df90e3877fc3e744e24c7a15d13 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Bill Farner



> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > RELEASE-NOTES.md
> > Line 4 (original), 4 (patched)
> > 
> >
> > This is a major change. We should proabably call it out here.

Done.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > api/src/main/thrift/org/apache/aurora/gen/storage.thrift
> > Line 149 (original)
> > 
> >
> > Can we remove that already? Could it be the case that we have some 
> > small scale users that run using the DB task store and still need that 
> > migration?

_Disclaimer - please push back unless you're fully convinced_

This is not necessary; the field is only used to avoid redundant work.

On master:
- `Snapshot.tasks` and `Snapshot.cronJobs` will always be *written*
- `Snapshot.tasks` and `Snapshot.cronJobs` will be *read* if any of:
  a. CLI option `useDbSnapshotForTaskStore = false`
  b. `Snapshot.dbScript` is empty
  c. `Snapshot.experimentalTaskStore = false`

This patch will always populate `Snapshot.tasks` and `Snapshot.cronJobs`, and 
will never populate `Snapshot.dbScript`.  As a result, the prior version will 
never skip over reading `Snapshot.tasks` or `Snapshot.cronJobs`.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
> > Lines 47 (patched)
> > 
> >
> > Nitpick: Inconsistent style.

Fixed.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
> > Lines 93-100 (original), 89-94 (patched)
> > 
> >
> > These comments are now outdated and should be removed.

Fixed, thanks!


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
> > Lines 145-162 (original), 130-131 (patched)
> > 
> >
> > How about we leave this in but allow it to be disabled with a flag? 
> > 
> > That way it would be easier to deploy this patch: If something goes 
> > wrong one can simply perform a rollback as the snapshot is completely 
> > compatible. If however we stop writing the db dump as done here, we would 
> > have to restore from a backup.
> > 
> > We could remove this once we drop the entire H2/mybatis stuff.

> If however we stop writing the db dump as done here, we would have to restore 
> from a backup

Please see my previous comment about compatibility, but i believe this is 
incorrect.  I've convinced myself that as long as `dbScript` is empty, all is 
well.  This is what motivated doing all stores at once - to avoid piecemeal 
removal of tables from `dbScript`.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
> > Lines 246-249 (original), 217 (patched)
> > 
> >
> > To check my understanding: There are three cases we have to handle:
> > 
> > a) db script but no host attributes in the snapshot -> we restore 
> > everything from the db script 
> > b) db script and host attributes in the snapshot -> the data is 
> > duplicated but consistent. Restoring it twice will do no harm
> > c) no db script any longer just host attributes in the snapshot: 
> > simple restore as seen here. Will be the code path used in the future.
> >   
> > Correct?

Correct, this is my understanding.


> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
> > Lines 41 (patched)
> > 
> >
> > Argument checking for stores is somewhat inconsistent. We should decide 
> > if we want them everywhere. If not, we should drop them.

Opting to remove based on popular majority within the code.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187581
---


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187610
---


Ship it!




Master (cb8e956) 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 Oct. 10, 2017, 11:16 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 11:16 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  b548fcdc4389af4420e57908a313435b5300445f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> 6e7f23d79deff4983b7feb568320cb938583270b 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> 202835debce817df82bd3d860330d23d9710f489 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> a59d1091b36d0b7908143335cf49d0dafb6627a1 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> 2fcd3e8f068c4ef950aabe52ca8050dcbd562500 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> c4e5fafc49028bd15cfe1bd2a874e73792091cb6 
>   commons/src/main/java/org/apache/aurora/common/args/TypeUtil.java 
> 80cbdd00ce2abb298232793c5fd20c83c6ba6de4 
>   commons/src/main/java/org/apache/aurora/common/args/Verifiers.java 
> 0212873258867ccdb17244e7b0678a7346e79b73 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecute.java
>  a26b8a2049a16e96fd97ad2163556de41ba5686e 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecuteFileVerifier.java
>  5d9b36070af7c717af5edcc0eec6774c2e26c102 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanRead.java 
> 3fef6a98e4c926126ac93061dd3b32c39f131e3c 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanReadFileVerifier.java
>  8c26304733264b6670e56ab032d7b039f4dfdbfe 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWrite.java 
> c2beeeb73d475ef9d03e1fde12e6d55983af81ed 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWriteFileVerifier.java
>  eac273868d72993bc3e83742398c050db2ca4aea 
>   commons/src/main/java/org/apache/aurora/common/args/constraints/Exists.java 
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187605
---



Master (cb8e956) is red with this patch.
  ./build-support/jenkins/build.sh

Caused by: java.lang.IllegalAccessException at CommandLineTest.java:431

org.apache.aurora.scheduler.config.CommandLineTest > testCustomOptions FAILED
java.lang.RuntimeException at CommandLineTest.java:76
Caused by: java.lang.IllegalAccessException at CommandLineTest.java:76
I1010 23:26:25.706 [ShutdownHook, SchedulerMain] Stopping scheduler services. 

1140 tests completed, 2 failed, 2 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:jacocoTestCoverageVerification[ant:jacocoReport] Rule violated for bundle 
aurora: branches covered ratio is 0.78, but expected minimum is 0.79
 FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/test/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':jacocoTestCoverageVerification'.
> Rule violated for bundle aurora: branches covered ratio is 0.78, but expected 
> minimum is 0.79

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

* Get more help at https://help.gradle.org

BUILD FAILED in 5m 55s
48 actionable tasks: 39 executed, 9 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 11:16 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 11:16 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  b548fcdc4389af4420e57908a313435b5300445f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Bill Farner


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/
---

(Updated Oct. 10, 2017, 4:16 p.m.)


Review request for Aurora, David McLaughlin and John Sirois.


Changes
---

make it easier to consume custom options


Repository: aurora


Description
---

This is a very big patch, lots of code removed.  I suggest starting with 
`CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on page 
8 verifies that every registered option can be parsed.

I made a similar effort a while back, but was unable to finish due to time 
constraints.  I'm now proposing a more drastic simplification in command line 
argument handling in the scheduler.  Less magic, more approachable, less 
brittle with other tools (gradle, IDEs).  The original approach made lots of 
sense in the original environment where Aurora was developed (a monorepo in a 
large organization), but this is no longer the case.

Historical context: 
https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E


Diffs (updated)
-

  build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
  commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
4da91591d325ab657dcd37325e5142c65db2ab8c 
  commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
25ed25093bd9defd78df741b6f51e0de0d1f1709 
  commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
a72ea7a1669a53d282f9a5a3709add506c9d4b33 
  commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
3366531cebe6a79d9c568322f9117d7dbc3e824d 
  commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
93c23234df38c9f917c0f582b51c25070f996c94 
  commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
e84d3029a45cf7ea1f0bb885788c677ed649b60e 
  commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
  commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
aad8807938f569d0dc7a970166ee71ea36f3537d 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
 5fda5dc6cd8b6d97511073830278850d58bb0bc7 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java 
1832d41feeafb07f4e7f0bef9dbcb6097e288508 
  
commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
 b548fcdc4389af4420e57908a313435b5300445f 
  commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
  commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
6e7f23d79deff4983b7feb568320cb938583270b 
  commons/src/main/java/org/apache/aurora/common/args/Args.java 
202835debce817df82bd3d860330d23d9710f489 
  commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
a59d1091b36d0b7908143335cf49d0dafb6627a1 
  commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
2fcd3e8f068c4ef950aabe52ca8050dcbd562500 
  commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
c4e5fafc49028bd15cfe1bd2a874e73792091cb6 
  commons/src/main/java/org/apache/aurora/common/args/TypeUtil.java 
80cbdd00ce2abb298232793c5fd20c83c6ba6de4 
  commons/src/main/java/org/apache/aurora/common/args/Verifiers.java 
0212873258867ccdb17244e7b0678a7346e79b73 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecute.java 
a26b8a2049a16e96fd97ad2163556de41ba5686e 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecuteFileVerifier.java
 5d9b36070af7c717af5edcc0eec6774c2e26c102 
  commons/src/main/java/org/apache/aurora/common/args/constraints/CanRead.java 
3fef6a98e4c926126ac93061dd3b32c39f131e3c 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanReadFileVerifier.java
 8c26304733264b6670e56ab032d7b039f4dfdbfe 
  commons/src/main/java/org/apache/aurora/common/args/constraints/CanWrite.java 
c2beeeb73d475ef9d03e1fde12e6d55983af81ed 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanWriteFileVerifier.java
 eac273868d72993bc3e83742398c050db2ca4aea 
  commons/src/main/java/org/apache/aurora/common/args/constraints/Exists.java 
217d10e561b175885d6bb2a058225935fd88b37a 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/ExistsFileVerifier.java
 e79f54701dea77440b585034790cc4cee987b99a 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/IsDirectory.java
 d909994937e5ab7f9e125bdf000c8a22c2f84734 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/IsDirectoryFileVerifier.java
 968a098357ad37cd6d6121fc049990ffea0fb2d8 
  commons/src/main/java/org/apache/aurora/common/args/constraints/NotEmpty.java 
8ff96afdfa2b837526956a77c8819c097c14881c

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Bill Farner



> On Oct. 10, 2017, 2:39 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
> > Line 143 (original), 146 (patched)
> > 
> >
> > Bill, totally unrelated question: Has anyone of you ever experimented 
> > with the TCompactProtocol rather than the TBinaryProtocol? 
> > 
> > I have heard rumors that it should be smaller while providing similar 
> > performance. So, it might be worth a try for our IO bound operations.

As it turns out, i recently experimented with this on my laptop to see how it 
would affect backup sizes.  IIRC it only shaved a few percent off the total 
backup size, and the difference disappeared when the backup was gzipped.  
Minimal enough that i did not investigate further.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187594
---


On Oct. 10, 2017, 1:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 1:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187597
---



Master (4a1fba3) 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 Oct. 10, 2017, 8:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 8:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Stephan Erb


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187595
---



@ReviewBot retry

- Stephan Erb


On Oct. 10, 2017, 10:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Stephan Erb

> On Oct. 10, 2017, 10:53 p.m., Jordan Ly wrote:
> > Change LGTM, but is build failing due to this change or being flaky?

Not enough memory on Jenkins :/

INFO: os::commit_memory(0x0007b110, 248512512, 0) failed; 
error='Cannot allocate memory' (errno=12)
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 248512512 bytes for 
committing reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid2009.log

I once added a hack to the Jenkins job to skip nodes with insufficient memory. 
Maybe it glitched here and failed to prevent the issue...

- Stephan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187586
---

On Oct. 10, 2017, 10:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Stephan Erb

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187581
---

I have a first set of comments, but will have to complete my review in a second 
sitting.

RELEASE-NOTES.md
Line 4 (original), 4 (patched)

This is a major change. We should proabably call it out here.

api/src/main/thrift/org/apache/aurora/gen/storage.thrift
Line 149 (original)

Can we remove that already? Could it be the case that we have some small 
scale users that run using the DB task store and still need that migration?

src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
Lines 47 (patched)

Nitpick: Inconsistent style.

src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
Lines 93-100 (original), 89-94 (patched)

These comments are now outdated and should be removed.

src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
Lines 145-162 (original), 130-131 (patched)

How about we leave this in but allow it to be disabled with a flag? 

That way it would be easier to deploy this patch: If something goes wrong 
one can simply perform a rollback as the snapshot is completely compatible. If 
however we stop writing the db dump as done here, we would have to restore from 
a backup.

We could remove this once we drop the entire H2/mybatis stuff.

src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
Lines 246-249 (original), 217 (patched)

To check my understanding: There are three cases we have to handle:

a) db script but no host attributes in the snapshot -> we restore 
everything from the db script 
b) db script and host attributes in the snapshot -> the data is 
duplicated but consistent. Restoring it twice will do no harm
c) no db script any longer just host attributes in the snapshot: simple 
restore as seen here. Will be the code path used in the future.

Correct?

src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
Lines 41 (patched)

Argument checking for stores is somewhat inconsistent. We should decide if 
we want them everywhere. If not, we should drop them.

- Stephan Erb

On Oct. 10, 2017, 9:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 9:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187591
---



Master (4a1fba3) 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 Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>   src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
> cac02a55d26b2934099a2b03457c703600296292 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> bb7055ed6826d57cb6b5c5c77e4a27c3bcdd10c7 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> 52c3c6618a3cf1009435ca8a9cece36365913e55 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 6c676693e531541ef9aec7f7a130c119ebf35df3 
>   src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java
>  bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
> 0a2516e4843b8f920700ece70b3cc816d2acecf0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
> 7904e3880d214aac1013ce18265fed924ef7897e 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 942d180fe1a8b7a583e812f9106ee0e8db1bea55 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
> 835f1604c0c5d913a87d570ee01d053bbbf92ecb 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 9d2164f4f2d18e4595d3039d64cedacb7df00ae2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Jordan Ly


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187586
---


Ship it!




Change LGTM, but is build failing due to this change or being flaky?

- Jordan Ly


On Oct. 10, 2017, 8:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 8:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62873: Stream backup file from disk

2017-10-10 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/#review187585
---

Master (4a1fba3) is red with this patch.
  ./build-support/jenkins/build.sh

#
INFO: os::commit_memory(0x0007b110, 248512512, 0) failed; error='Cannot 
allocate memory' (errno=12)
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 248512512 bytes for committing 
reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid2009.log
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:jacocoTestCoverageVerification[ant:jacocoReport] Rule violated for bundle 
aurora: instructions covered ratio is 0.76, but expected minimum is 0.87
[ant:jacocoReport] Rule violated for bundle aurora: branches covered ratio is 
0.65, but expected minimum is 0.79
 FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> Process 'Gradle Test Executor 8' finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':jacocoTestCoverageVerification'.
> Rule violated for bundle aurora: instructions covered ratio is 0.76, but 
> expected minimum is 0.87
  Rule violated for bundle aurora: branches covered ratio is 0.65, but expected 
minimum is 0.79

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

* Get more help at https://help.gradle.org

BUILD FAILED in 9m 26s
50 actionable tasks: 41 executed, 9 up-to-date

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot

On Oct. 10, 2017, 4:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62873/
> ---
> 
> (Updated Oct. 10, 2017, 4:35 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reduces the memory burden of loading a backup for recovery.  Previously, 
> the backup file would be fully loaded into a `byte[]`, which may be very 
> large and fail to allocate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
> b74de9b51fd01788a3970e56a534a3b9adcd8863 
> 
> 
> Diff: https://reviews.apache.org/r/62873/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 62873: Stream backup file from disk

2017-10-10 Thread Bill Farner


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62873/
---

Review request for Aurora and Jordan Ly.


Repository: aurora


Description
---

This reduces the memory burden of loading a backup for recovery.  Previously, 
the backup file would be fully loaded into a `byte[]`, which may be very large 
and fail to allocate.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 
b74de9b51fd01788a3970e56a534a3b9adcd8863 


Diff: https://reviews.apache.org/r/62873/diff/1/


Testing
---


Thanks,

Bill Farner

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread David McLaughlin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187575
---


Ship it!




Ship It!

- David McLaughlin


On Oct. 10, 2017, 2:37 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 2:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  b548fcdc4389af4420e57908a313435b5300445f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> 6e7f23d79deff4983b7feb568320cb938583270b 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> 202835debce817df82bd3d860330d23d9710f489 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> a59d1091b36d0b7908143335cf49d0dafb6627a1 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> 2fcd3e8f068c4ef950aabe52ca8050dcbd562500 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> c4e5fafc49028bd15cfe1bd2a874e73792091cb6 
>   commons/src/main/java/org/apache/aurora/common/args/TypeUtil.java 
> 80cbdd00ce2abb298232793c5fd20c83c6ba6de4 
>   commons/src/main/java/org/apache/aurora/common/args/Verifiers.java 
> 0212873258867ccdb17244e7b0678a7346e79b73 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecute.java
>  a26b8a2049a16e96fd97ad2163556de41ba5686e 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecuteFileVerifier.java
>  5d9b36070af7c717af5edcc0eec6774c2e26c102 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanRead.java 
> 3fef6a98e4c926126ac93061dd3b32c39f131e3c 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanReadFileVerifier.java
>  8c26304733264b6670e56ab032d7b039f4dfdbfe 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWrite.java 
> c2beeeb73d475ef9d03e1fde12e6d55983af81ed 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWriteFileVerifier.java
>  eac273868d72993bc3e83742398c050db2ca4aea 
>   commons/src/main/java/org/apache/aurora/common/args/constraints/Exists.java 
> 217d10e561b175885d6bb2a058225935fd88b37a 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/ExistsFileVerifier.java
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/#review187574
---



Master (4a1fba3) is red with this patch.
  ./build-support/jenkins/build.sh

 @ ./~/moment/src/lib/locale/locales.js 65:16-60
 @ ./~/moment/src/lib/locale/locale.js
 @ ./~/moment/src/moment.js
 @ ./src/main/js/utils/Task.js
 @ ./src/main/js/pages/Instance.js
 @ ./src/main/js/index.js
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileTestJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java:71:
 'org.apache.aurora.scheduler.storage.Util.jobUpdateActionStatName' should be 
separated from previous imports. [ImportOrder]
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 2m 57s
37 actionable tasks: 31 executed, 6 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 7:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for transaction atomicity.
> 
> See the [mailing list 
> discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
>  for context.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 9ee9eff6c3d657decd93458dc3e6792a30614a60 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
>   src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
> ae59f3def75d0e1d3866c8d2f85063456643e9a6 
>   src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
> b4f14f1f352dbe56afe57d66349ae5ae57533050 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> c81387f24d554bcb2ee73e49028ba068ad11e4d6 
>   src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
> 5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 440c4fc551905da513df5dae21d0eefe6a42ceeb 
>

Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Bill Farner


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62869/
---

Review request for Aurora, Jordan Ly and Stephan Erb.


Repository: aurora


Description
---

This patch introduces map-based volatile stores, most of which were revived 
from git history with minimal changes.  The DB storage system is now only used 
in a temporary storage when replaying a snapshot containing the `dbScript` 
field.

Please take special care to double-check my work in `SnapshotStoreImpl`, as it 
must function for backwards compatibility with `Snapshot`s in the wild.  
Another area of interest is the implementation of `MemJobUpdateStore`, which 
must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most of 
this stems from behind-the-scenes interaction with `LockStore` and use of 
cascading deletes.

Note that this change removes the transactional nature of in-memory storage 
operations as well as the `READ COMMITTED` transaction isolation previously 
available to some stores (proven in necessary changes to 
`StorageTransactionTest`).  This means some stores will permit dirty reads when 
they previously did not.  `TaskStore` has always had this non-transactional 
behavior by default, as the DB task store was never deemed suitable for 
production.  Nonetheless, this non-transactional behavior should be considered 
safe as the scheduler fails over on a storage operation failure, and relies on 
the persistent log storage for transaction atomicity.

See the [mailing list 
discussion](https://lists.apache.org/thread.html/0bb74c78da7fa12954e2438e8011b3bff73fe3bfdafeaaa2ff00a98e@%3Cdev.aurora.apache.org%3E)
 for context.


Diffs
-

  RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
  api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
9ee9eff6c3d657decd93458dc3e6792a30614a60 
  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  examples/vagrant/upstart/aurora-scheduler.conf 
63fcc87be653835cb3c3f25dae4164f1d7c8d4da 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
cb783ce4e4b40f173fb3b1cbd0094a5a4e0300a5 
  src/jmh/java/org/apache/aurora/benchmark/SnapshotBenchmarks.java 
ae59f3def75d0e1d3866c8d2f85063456643e9a6 
  src/jmh/java/org/apache/aurora/benchmark/StateManagerBenchmarks.java 
b4f14f1f352dbe56afe57d66349ae5ae57533050 
  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
c81387f24d554bcb2ee73e49028ba068ad11e4d6 
  src/jmh/java/org/apache/aurora/benchmark/TaskStoreBenchmarks.java 
5b4d2a267b9cc1a14b915a1a10d63b4f4d174d51 
  src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
440c4fc551905da513df5dae21d0eefe6a42ceeb 
  src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java 
cac02a55d26b2934099a2b03457c703600296292 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
bb7055ed6826d57cb6b5c5c77e4a27c3bcdd10c7 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
52c3c6618a3cf1009435ca8a9cece36365913e55 
  src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
6c676693e531541ef9aec7f7a130c119ebf35df3 
  src/main/java/org/apache/aurora/scheduler/storage/Util.java PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 
bad05f5bf1976bb590e8dd7af9b43d5d30e892a9 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
cbe5a0deff1ebc38a9618e7d89ab073dfaf78d36 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
0a2516e4843b8f920700ece70b3cc816d2acecf0 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
7904e3880d214aac1013ce18265fed924ef7897e 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
942d180fe1a8b7a583e812f9106ee0e8db1bea55 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
835f1604c0c5d913a87d570ee01d053bbbf92ecb 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
9d2164f4f2d18e4595d3039d64cedacb7df00ae2 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
d145259653dd4df90e3877fc3e744e24c7a15d13 
  src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 
1b491f977cf3a81e61f1333082be0547420306d4 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemSchedulerStore.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorage.java 
PRE-CREATION

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187560
---



Master (2df250e) is red with this patch.
  ./build-support/jenkins/build.sh

 
.pants.d/python-setup/chroots/ba32407587069aafa8e32c641525ba88361a07d7/.deps/mock-1.0.1-py2-none-any.whl/mock.py:1201:
 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 mocks = (, 
, , , )
 td = '/tmp/tmpmUI8jr'
 some_user = pwd.struct_passwd(pw_name='jglick', 
pw_passwd='*', pw_uid=1939, pw_gid=1939, pw_gecos='Jesse N. Glick', 
pw_dir='/home/jglick', pw_shell='/usr/local/bin/bash')
 taskpath = 
 
 [1m@mock.patch('os.chown')[0m
 [1m@mock.patch('os.setgroups')[0m
 [1m@mock.patch('os.setgid')[0m
 [1m@mock.patch('os.setuid')[0m
 [1m@mock.patch('os.geteuid', return_value=0)[0m
 [1mdef test_log_permissions_other_user(*mocks):[0m
 [1m  with temporary_dir() as td:[0m
 [1msome_user = get_other_nonroot_user()[0m
 [1mtaskpath = make_taskpath(td)[0m
 [1msandbox = setup_sandbox(td, taskpath)[0m
 [1m[0m
 [1mp = TestProcess('process', 'echo hello world', 
0, taskpath, sandbox, user=some_user.pw_name)[0m
 [1mp.start()[0m
 [1m
wait_for_rc(taskpath.getpath('process_checkpoint'))[0m
 [1m[0m
 [1m# since we're not actually root, the best we 
can do is check the right things were attempted[0m
 [1mstdout = 
taskpath.with_filename('stdout').getpath('process_logdir')[0m
 [1mstderr = 
taskpath.with_filename('stderr').getpath('process_logdir')[0m
 [1m>   assert os.path.exists(stdout)[0m
 [1m[31mE   assert ('/tmp/tmpmUI8jr/.logs/process/0/stdout')[0m
 [1m[31mE+  where  = .exists[0m
 [1m[31mE+where  = os.path[0m
 
 src/test/python/apache/thermos/core/test_process.py:264: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/aaf4d108c31293299a0839bdc404a91802f80937.xml
 
 [1m[31m 1 failed, 795 passed, 6 skipped, 1 warnings in 
318.42 seconds [0m
 
FAILURE


17:35:22 05:54   [complete][31m
   FAILURE[0m


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 5:02 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 10, 2017, 5:02 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
>   ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 50d63e6252b38cf26688ab032409a00082561f14 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread Santhosh Kumar Shanmugham


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187558
---


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 10, 2017, 10:02 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 10, 2017, 10:02 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
>   ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 50d63e6252b38cf26688ab032409a00082561f14 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 8f077341b2194fc00d202b0eb289b23986d0f2e0 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js be8766c3b53943dcf094670019ec528975dc05b5 
>   ui/src/main/js/utils/Thrift.js b247e36dfa7e8b686e8e6caccfec9ff813829bab 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss e301d4c31baed10faf06d84f30d1c1dbce66952a 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 4ebec0532550a0c6084fb603cfab5789c1ccb371 
>   ui/src/main/sass/components/_update-list.scss PRE-CREATION 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js a403434ddd7d6b33b7e18ca8c25ab68707a35fd8 
> 
> 
> Diff: https://reviews.apache.org/r/62763/diff/3/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> Tested against live APIs. Note: I am pushing small nitpick bugs to the very 
> end of this work (where we will leverage internal beta-testing).
> 
> 
> File Attachments
> 
> 
> Successful update
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
> Update In-Progress
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
> Huge update
>   
>

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread David McLaughlin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/
---

(Updated Oct. 10, 2017, 5:02 p.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Changes
---

feedback.


Repository: aurora


Description
---

Implement update page for new UI. 

The most significant change here is the performance - for our jobs at Twitter 
with thousands of instances, this shaves tens of seconds off the render time 
compared to the Angular version (which I think is attributed to the tooltip 
code). 

I translated the "instance events" into something useful - now grouping them 
all events for an instance together in a single item and showing history with 
the state machine component. 

For the instance visualization, I also simplified the color scheme. There is 
now only: success, error, warning, in progress and removed. So no more roll 
backs being represented as green boxes with red borders (roll backs are now 
considered as the warning/attention color). 

I didn't do much for configuration and kept the UX the same as the existing UI, 
I will tackle showing diffs in the future. 

This is branched off of https://reviews.apache.org/r/62720


Diffs (updated)
-

  ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
  ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
  ui/src/main/js/components/InstanceViz.js PRE-CREATION 
  ui/src/main/js/components/Layout.js 50d63e6252b38cf26688ab032409a00082561f14 
  ui/src/main/js/components/Pagination.js 
7bf2c04e2b879657c6ec43d261f2512fae79a08e 
  ui/src/main/js/components/TaskConfig.js PRE-CREATION 
  ui/src/main/js/components/Time.js PRE-CREATION 
  ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
  ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
  ui/src/main/js/components/UpdateList.js PRE-CREATION 
  ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
  ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
  ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
  ui/src/main/js/components/UpdateTime.js PRE-CREATION 
  ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Pagination-test.js 
f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
  ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
  ui/src/main/js/index.js 8f077341b2194fc00d202b0eb289b23986d0f2e0 
  ui/src/main/js/pages/Update.js PRE-CREATION 
  ui/src/main/js/pages/Updates.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
  ui/src/main/js/utils/Common.js be8766c3b53943dcf094670019ec528975dc05b5 
  ui/src/main/js/utils/Thrift.js b247e36dfa7e8b686e8e6caccfec9ff813829bab 
  ui/src/main/js/utils/Update.js PRE-CREATION 
  ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/sass/app.scss e301d4c31baed10faf06d84f30d1c1dbce66952a 
  ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
  ui/src/main/sass/components/_layout.scss 
4ebec0532550a0c6084fb603cfab5789c1ccb371 
  ui/src/main/sass/components/_update-list.scss PRE-CREATION 
  ui/src/main/sass/components/_update-page.scss PRE-CREATION 
  ui/test-setup.js a403434ddd7d6b33b7e18ca8c25ab68707a35fd8 


Diff: https://reviews.apache.org/r/62763/diff/3/

Changes: https://reviews.apache.org/r/62763/diff/2-3/


Testing
---

./gradlew ui:lint
./gradlew ui:test

Tested against live APIs. Note: I am pushing small nitpick bugs to the very end 
of this work (where we will leverage internal beta-testing).


File Attachments


Successful update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
Update In-Progress
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
Huge update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/5b2534e3-e5d6-48e8-9615-2b1f7bb13617__Screen_Shot_2017-10-04_at_5.27.15_PM.png


Thanks,

David McLaughlin

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread David McLaughlin



> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/__tests__/InstanceViz-test.js
> > Lines 23 (patched)
> > 
> >
> > nit - assert `medium` and `big` do not appear?

Done.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/__tests__/Pagination-test.js
> > Lines 78 (patched)
> > 
> >
> > We do not sort when `sortBy` is omitted. What do you mean by native 
> > sort?

The natural order of the collection (i.e. order by array index). Updated 
description.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/utils/__tests__/Update-test.js
> > Lines 47 (patched)
> > 
> >
> > s/success/inprogress/

Fixed.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/client/scheduler-client.js
> > Line 2 (original), 2 (patched)
> > 
> >
> > Drop this?

Fixed.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/UpdateInstanceEvents.js
> > Lines 50 (patched)
> > 
> >
> > The `events` that are pushed into this component are already sorted. Do 
> > we require an extra sort?

In the parent component, they are *reverse* sorted (to show a descending list 
of latest instances updated) and then here they are sorted in ascending order 
(for the state machine).


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187281
---


On Oct. 5, 2017, 10:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 10:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187550
---


Ship it!




Master (0169b81) 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 Oct. 10, 2017, 2:37 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 2:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  b548fcdc4389af4420e57908a313435b5300445f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> 6e7f23d79deff4983b7feb568320cb938583270b 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> 202835debce817df82bd3d860330d23d9710f489 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> a59d1091b36d0b7908143335cf49d0dafb6627a1 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> 2fcd3e8f068c4ef950aabe52ca8050dcbd562500 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> c4e5fafc49028bd15cfe1bd2a874e73792091cb6 
>   commons/src/main/java/org/apache/aurora/common/args/TypeUtil.java 
> 80cbdd00ce2abb298232793c5fd20c83c6ba6de4 
>   commons/src/main/java/org/apache/aurora/common/args/Verifiers.java 
> 0212873258867ccdb17244e7b0678a7346e79b73 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecute.java
>  a26b8a2049a16e96fd97ad2163556de41ba5686e 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecuteFileVerifier.java
>  5d9b36070af7c717af5edcc0eec6774c2e26c102 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanRead.java 
> 3fef6a98e4c926126ac93061dd3b32c39f131e3c 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanReadFileVerifier.java
>  8c26304733264b6670e56ab032d7b039f4dfdbfe 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWrite.java 
> c2beeeb73d475ef9d03e1fde12e6d55983af81ed 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/CanWriteFileVerifier.java
>  eac273868d72993bc3e83742398c050db2ca4aea 
>   commons/src/main/java/org/apache/aurora/common/args/constraints/Exists.java 
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187547
---



Master (0169b81) is red with this patch.
  ./build-support/jenkins/build.sh

:licenseTest UP-TO-DATE
:license UP-TO-DATE
:pmdJmh
:pmdMain
:pmdTest
:testI1010 15:31:35.753 [ShutdownHook, SchedulerMain] Stopping scheduler 
services. 
 FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:jacocoTestCoverageVerification[ant:jacocoReport] Rule violated for bundle 
aurora: instructions covered ratio is 0.81, but expected minimum is 0.87
[ant:jacocoReport] Rule violated for bundle aurora: branches covered ratio is 
0.69, but expected minimum is 0.79
 FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> Process 'Gradle Test Executor 8' finished with non-zero exit value 137

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':jacocoTestCoverageVerification'.
> Rule violated for bundle aurora: instructions covered ratio is 0.81, but 
> expected minimum is 0.87
  Rule violated for bundle aurora: branches covered ratio is 0.69, but expected 
minimum is 0.79

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

* Get more help at https://help.gradle.org

BUILD FAILED in 20m 23s
48 actionable tasks: 39 executed, 9 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 2:37 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 2:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>  b548fcdc4389af4420e57908a313435b5300445f 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen



> On Oct. 10, 2017, 2:29 p.m., Joshua Cohen wrote:
> > I'd be shocked if they've been failing for over a year. We've had three 
> > releases since then and the release candidate verification script runs the 
> > end to end tests.
> > 
> > If I had to hazard a guess, I'd point the finger at something changing in 
> > curl w/ the High Sierra update? I'm running the e2e tests locally right now 
> > (I'm still on Sierra) to confirm they're working there.
> 
> Joshua Cohen wrote:
> Oh wait, the curl version being used is from within the Vagrant image, 
> not the host system. So... perhaps something changed in the curl version in 
> vagrant?

I was just able to verify the failure. So it's not specific to your system or 
host OS. It'd be interesting to bisect to see what actually caused the failure, 
but either way your fix lgtm.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187535
---


On Oct. 10, 2017, 6:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 6:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> `Unsupported Content-Type: application/x-www-form-urlencoded`, which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Stephan Erb


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187542
---


Ship it!




Ship It!

- Stephan Erb


On Oct. 10, 2017, 8:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 8:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Stephan Erb



> On Oct. 10, 2017, 4:59 p.m., Stephan Erb wrote:
> > I have been running the end-to-end tests from time to time and have not 
> > seen this problem, yet.
> > 
> > Code wide, I also don't think this should be necessary. We are defaulting 
> > to json if there is no accept header in order to maintain compatibility. 
> > See [1] and [2]
> > 
> > [1] 
> > https://github.com/apache/aurora/commit/0105a15#diff-7b5266c03f44c6cec3de918741c37171R108
> > [2] 
> > https://github.com/apache/aurora/commit/0105a15#diff-8c860297930ef9ce9d2e9885f577b2b8R115

Oh, I have not been reading your supplied error properly. 

Maybe I am running with an old vagrant box that has not seen the latest updates?


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187540
---


On Oct. 10, 2017, 8:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 8:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Stephan Erb


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187540
---



I have been running the end-to-end tests from time to time and have not seen 
this problem, yet.

Code wide, I also don't think this should be necessary. We are defaulting to 
json if there is no accept header in order to maintain compatibility. See [1] 
and [2]

[1] 
https://github.com/apache/aurora/commit/0105a15#diff-7b5266c03f44c6cec3de918741c37171R108
[2] 
https://github.com/apache/aurora/commit/0105a15#diff-8c860297930ef9ce9d2e9885f577b2b8R115

- Stephan Erb


On Oct. 10, 2017, 8:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 8:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/#review187539
---



Master (0169b81) is red with this patch.
  ./build-support/jenkins/build.sh

#
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 232783872 bytes for committing 
reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid31468.log
I1010 14:49:58.885 [ShutdownHook, SchedulerMain] Stopping scheduler services. 
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:jacocoTestCoverageVerification[ant:jacocoReport] Rule violated for bundle 
aurora: instructions covered ratio is 0.80, but expected minimum is 0.87
[ant:jacocoReport] Rule violated for bundle aurora: branches covered ratio is 
0.69, but expected minimum is 0.79
 FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':test'.
> Process 'Gradle Test Executor 8' finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

2: Task failed with an exception.
---
* What went wrong:
Execution failed for task ':jacocoTestCoverageVerification'.
> Rule violated for bundle aurora: instructions covered ratio is 0.80, but 
> expected minimum is 0.87
  Rule violated for bundle aurora: branches covered ratio is 0.69, but expected 
minimum is 0.79

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

* Get more help at https://help.gradle.org

BUILD FAILED in 7m 28s
48 actionable tasks: 39 executed, 9 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 2:37 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62623/
> ---
> 
> (Updated Oct. 10, 2017, 2:37 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a very big patch, lots of code removed.  I suggest starting with 
> `CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on 
> page 8 verifies that every registered option can be parsed.
> 
> I made a similar effort a while back, but was unable to finish due to time 
> constraints.  I'm now proposing a more drastic simplification in command line 
> argument handling in the scheduler.  Less magic, more approachable, less 
> brittle with other tools (gradle, IDEs).  The original approach made lots of 
> sense in the original environment where Aurora was developed (a monorepo in a 
> large organization), but this is no longer the case.
> 
> Historical context: 
> https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E
> 
> 
> Diffs
> -
> 
>   build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
>   commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
> 4da91591d325ab657dcd37325e5142c65db2ab8c 
>   commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
> 25ed25093bd9defd78df741b6f51e0de0d1f1709 
>   commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
> a72ea7a1669a53d282f9a5a3709add506c9d4b33 
>   commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
> 3366531cebe6a79d9c568322f9117d7dbc3e824d 
>   commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
> 93c23234df38c9f917c0f582b51c25070f996c94 
>   commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
> e84d3029a45cf7ea1f0bb885788c677ed649b60e 
>   commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
> 8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
>   commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
> aad8807938f569d0dc7a970166ee71ea36f3537d 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5fda5dc6cd8b6d97511073830278850d58bb0bc7 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  1832d41feeafb07f4e7f0bef9dbcb6097e288508 
>   
> commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
>

Re: Review Request 62623: Use a simpler command line argument system

2017-10-10 Thread Bill Farner


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62623/
---

(Updated Oct. 10, 2017, 7:37 a.m.)


Review request for Aurora, David McLaughlin and John Sirois.


Changes
---

- added and/or ported converters where needed
- added option validators
- added a comprehensive test


Repository: aurora


Description (updated)
---

This is a very big patch, lots of code removed.  I suggest starting with 
`CliOptions.java` and `CommandLine.java` on page 5.  `CommandLIneTest` on page 
8 verifies that every registered option can be parsed.

I made a similar effort a while back, but was unable to finish due to time 
constraints.  I'm now proposing a more drastic simplification in command line 
argument handling in the scheduler.  Less magic, more approachable, less 
brittle with other tools (gradle, IDEs).  The original approach made lots of 
sense in the original environment where Aurora was developed (a monorepo in a 
large organization), but this is no longer the case.

Historical context: 
https://mail-archives.apache.org/mod_mbox/aurora-dev/201601.mbox/%3CCAFWq12VJPOxDViDP5OsmXU3skHR6rrp5Zp%2BXgi8HgeHic6pTJw%40mail.gmail.com%3E


Diffs (updated)
-

  build.gradle d7a4287a4a073aedb905af57309ed08f889eea0b 
  commons-args/src/main/java/org/apache/aurora/common/args/Arg.java 
4da91591d325ab657dcd37325e5142c65db2ab8c 
  commons-args/src/main/java/org/apache/aurora/common/args/ArgParser.java 
25ed25093bd9defd78df741b6f51e0de0d1f1709 
  commons-args/src/main/java/org/apache/aurora/common/args/CmdLine.java 
a72ea7a1669a53d282f9a5a3709add506c9d4b33 
  commons-args/src/main/java/org/apache/aurora/common/args/NoParser.java 
3366531cebe6a79d9c568322f9117d7dbc3e824d 
  commons-args/src/main/java/org/apache/aurora/common/args/Parser.java 
93c23234df38c9f917c0f582b51c25070f996c94 
  commons-args/src/main/java/org/apache/aurora/common/args/ParserOracle.java 
e84d3029a45cf7ea1f0bb885788c677ed649b60e 
  commons-args/src/main/java/org/apache/aurora/common/args/Verifier.java 
8f09a9803b8a05b2ab11326d48f26cccaaa278d1 
  commons-args/src/main/java/org/apache/aurora/common/args/VerifierFor.java 
aad8807938f569d0dc7a970166ee71ea36f3537d 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
 5fda5dc6cd8b6d97511073830278850d58bb0bc7 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java 
1832d41feeafb07f4e7f0bef9dbcb6097e288508 
  
commons-args/src/main/resources/META-INF/services/javax.annotation.processing.Processor
 b548fcdc4389af4420e57908a313435b5300445f 
  commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
9fd6eaec98516c3ca9b9a8647af848fcc96509bd 
  commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
6e7f23d79deff4983b7feb568320cb938583270b 
  commons/src/main/java/org/apache/aurora/common/args/Args.java 
202835debce817df82bd3d860330d23d9710f489 
  commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
a59d1091b36d0b7908143335cf49d0dafb6627a1 
  commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
2fcd3e8f068c4ef950aabe52ca8050dcbd562500 
  commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
c4e5fafc49028bd15cfe1bd2a874e73792091cb6 
  commons/src/main/java/org/apache/aurora/common/args/TypeUtil.java 
80cbdd00ce2abb298232793c5fd20c83c6ba6de4 
  commons/src/main/java/org/apache/aurora/common/args/Verifiers.java 
0212873258867ccdb17244e7b0678a7346e79b73 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecute.java 
a26b8a2049a16e96fd97ad2163556de41ba5686e 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanExecuteFileVerifier.java
 5d9b36070af7c717af5edcc0eec6774c2e26c102 
  commons/src/main/java/org/apache/aurora/common/args/constraints/CanRead.java 
3fef6a98e4c926126ac93061dd3b32c39f131e3c 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanReadFileVerifier.java
 8c26304733264b6670e56ab032d7b039f4dfdbfe 
  commons/src/main/java/org/apache/aurora/common/args/constraints/CanWrite.java 
c2beeeb73d475ef9d03e1fde12e6d55983af81ed 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/CanWriteFileVerifier.java
 eac273868d72993bc3e83742398c050db2ca4aea 
  commons/src/main/java/org/apache/aurora/common/args/constraints/Exists.java 
217d10e561b175885d6bb2a058225935fd88b37a 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/ExistsFileVerifier.java
 e79f54701dea77440b585034790cc4cee987b99a 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/IsDirectory.java
 d909994937e5ab7f9e125bdf000c8a22c2f84734 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/IsDirectoryFileVerifier.java
 968a098357ad37cd6d6121fc049990ffea0fb2d8 
  commons/src/main/java/org/apache/aurora/common/args/constraints/NotEmpty.java

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen



> On Oct. 10, 2017, 2:29 p.m., Joshua Cohen wrote:
> > I'd be shocked if they've been failing for over a year. We've had three 
> > releases since then and the release candidate verification script runs the 
> > end to end tests.
> > 
> > If I had to hazard a guess, I'd point the finger at something changing in 
> > curl w/ the High Sierra update? I'm running the e2e tests locally right now 
> > (I'm still on Sierra) to confirm they're working there.

Oh wait, the curl version being used is from within the Vagrant image, not the 
host system. So... perhaps something changed in the curl version in vagrant?


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187535
---


On Oct. 10, 2017, 6:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 6:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187536
---


Ship it!




Regardless... ship away!

- Joshua Cohen


On Oct. 10, 2017, 6:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 6:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Joshua Cohen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187535
---



I'd be shocked if they've been failing for over a year. We've had three 
releases since then and the release candidate verification script runs the end 
to end tests.

If I had to hazard a guess, I'd point the finger at something changing in curl 
w/ the High Sierra update? I'm running the e2e tests locally right now (I'm 
still on Sierra) to confirm they're working there.

- Joshua Cohen


On Oct. 10, 2017, 6:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 6:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Bill Farner


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/
---

Review request for Aurora, David McLaughlin and Stephan Erb.


Repository: aurora


Description
---

TContentAwareServlet constrains the supported Content-Type headers,
resulting in test_kerberos_end_to_end.sh failing with the error
"Unsupported Content-Type: application/x-www-form-urlencoded", which
is the Content-Type header curl chooses when the --data-binary
argument is passed


Diffs
-

  src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
ecb3d3f50e77b800eb0d05618c7740c42fca28c2 


Diff: https://reviews.apache.org/r/62857/diff/1/


Testing
---

./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Based on what i'm seeing, it appears that these tests should have been failing 
since the introduction of `TContentAwareServlet` 
[0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
that true, or have others actually seen them passing somehow?


Thanks,

Bill Farner

Re: Review Request 62857: Fix broken end-to-end tests

2017-10-10 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62857/#review187502
---


Ship it!




Master (0169b81) 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 Oct. 10, 2017, 6:21 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62857/
> ---
> 
> (Updated Oct. 10, 2017, 6:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> TContentAwareServlet constrains the supported Content-Type headers,
> resulting in test_kerberos_end_to_end.sh failing with the error
> "Unsupported Content-Type: application/x-www-form-urlencoded", which
> is the Content-Type header curl chooses when the --data-binary
> argument is passed
> 
> 
> Diffs
> -
> 
>   src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
> ecb3d3f50e77b800eb0d05618c7740c42fca28c2 
> 
> 
> Diff: https://reviews.apache.org/r/62857/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Based on what i'm seeing, it appears that these tests should have been 
> failing since the introduction of `TContentAwareServlet` 
> [0105a15](https://github.com/apache/aurora/commit/0105a15) in Aug 2016.  Is 
> that true, or have others actually seen them passing somehow?
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62873: Stream backup file from disk

Re: Review Request 62873: Stream backup file from disk

Re: Review Request 62873: Stream backup file from disk

Re: Review Request 62873: Stream backup file from disk

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Re: Review Request 62873: Stream backup file from disk

Re: Review Request 62873: Stream backup file from disk

Review Request 62873: Stream backup file from disk

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

Re: Review Request 62763: Create Update Page with React

Re: Review Request 62763: Create Update Page with React

Re: Review Request 62763: Create Update Page with React

Re: Review Request 62763: Create Update Page with React

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62623: Use a simpler command line argument system

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

Review Request 62857: Fix broken end-to-end tests

Re: Review Request 62857: Fix broken end-to-end tests

34 matches

Site Navigation

Mail list logo

Footer information