Re: Review Request 63135: Refactor Job Page to make it more pluggable

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63135/#review188638 --- Master (0622f34) is red with this patch.

Re: Review Request 63135: Refactor Job Page to make it more pluggable

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63135/#review188637 --- @ReviewBot retry - David McLaughlin On Oct. 19, 2017, 4 a.m.,

Re: Review Request 63135: Refactor Job Page to make it more pluggable

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63135/#review188636 --- Master (0622f34) is red with this patch.

Review Request 63135: Refactor Job Page to make it more pluggable

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63135/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.

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

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188627 --- Master (c638877) is green with this patch.

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63121/#review188626 --- Ship it! Master (c638877) is green with this patch.

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188623 --- Ship it! Master (c638877) is green with this patch.

Re: Review Request 63130: Use LockStore only for backwards compatibility

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63130/#review188621 --- Ship it! Master (c638877) is green with this patch.

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/#review188617 --- Master (c638877) is green with this patch.

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-10-18 Thread Bill Farner
> On Oct. 18, 2017, 12:38 p.m., Nicolás Donatucci wrote: > > I found an issue a while back after talking to Joshua. > > It is regarding the downgrade script for my db migration. > > It works just fine assuming there is only one entry of ram, cpu and disk > > per taskId on the task_resource

Re: Review Request 63132: Cosmetic changes to Navigation and task metadata

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63132/#review188612 --- Ship it! Master (c638877) is green with this patch.

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Bill Farner
> On Oct. 18, 2017, 2:48 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/HostOffer.java > > Lines 44-47 (patched) > > > > > > Please extend this comment a bit and indicate that this checks for

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63121/ --- (Updated Oct. 18, 2017, 5:04 p.m.) Review request for Aurora and Jordan Ly.

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Bill Farner
> On Oct. 18, 2017, 2 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 203-205 (original), 204-206 (patched) > > > > > > By default, `min_offer_hold_time` is

Review Request 63130: Use LockStore only for backwards compatibility

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63130/ --- Review request for Aurora, Jordan Ly and Stephan Erb. Repository: aurora

Review Request 63132: Cosmetic changes to Navigation and task metadata

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63132/ --- Review request for Aurora and Santhosh Kumar Shanmugham. Repository: aurora

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/#review188605 --- Ship it! Ship It! - Jordan Ly On Oct. 8, 2017, 5:29 p.m.,

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/#review188601 --- Ship it! Ship It! - Jordan Ly On Oct. 8, 2017, 5:29 p.m.,

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188604 --- Ship it! Master (4f52f75) is green with this patch.

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188602 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 18,

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/#review188603 --- Ship it! Ship It! - Jordan Ly On Oct. 8, 2017, 5:29 p.m.,

Re: Review Request 63129: Fix lint build error

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63129/#review188600 --- Ship it! Master (4f52f75) is green with this patch.

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188599 --- @ReviewBot retry - David McLaughlin On Oct. 18, 2017, 9:40

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188598 --- Master (4f52f75) is red with this patch.

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188597 --- @ReviewBot retry - David McLaughlin On Oct. 18, 2017, 9:40

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188596 --- Master (ff39120) is red with this patch.

Re: Review Request 63129: Fix lint build error

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63129/#review188595 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 18,

Review Request 63129: Fix lint build error

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63129/ --- Review request for Aurora and Santhosh Kumar Shanmugham. Repository: aurora

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188593 --- Master (ff39120) is red with this patch.

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread David McLaughlin
> On Oct. 18, 2017, 9:48 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/HostOffer.java > > Lines 62-66 (patched) > > > > > > I have had a short peek at our clusters and the number of offers >

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188589 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 18,

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188588 --- Ship it! Ship It! - Jordan Ly On Oct. 18, 2017, 9:25 p.m.,

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/ --- (Updated Oct. 18, 2017, 9:40 p.m.) Review request for Aurora, Kai Huang and

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188585 --- Ship it! Ship It! - David McLaughlin On Oct. 18, 2017, 9:25

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread David McLaughlin
> On Oct. 18, 2017, 9:29 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/HostOffer.java > > Lines 64-65 (patched) > > > > > > Shouldn't this be or (||)? Oh, derp, nm. I read it backwards

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Stephan Erb
> On Oct. 13, 2017, 10:12 a.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 220-224 (patched) > > > > > > This won't work for us. > > > > We are using

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188582 --- src/main/java/org/apache/aurora/scheduler/HostOffer.java Lines

Re: Review Request 62956: When scheduling, skip offers with no CPU and no mem

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/ --- (Updated Oct. 18, 2017, 2:25 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/#review188581 --- Master (f820b27) is red with this patch.

Re: Review Request 63122: Remove corrupted fonts and add font files to .gitattributes to prevent line-ending formatting by git

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63122/#review188579 --- Ship it! Master (f820b27) is green with this patch.

Review Request 63125: Add cron configuration to Job Page

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63125/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63121/#review188578 ---

Re: Review Request 63117: Role and Environment Page fixes

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63117/#review188575 --- Ship it! Master (b7e83a0) is green with this patch.

Re: Review Request 63122: Remove corrupted fonts and add font files to .gitattributes to prevent line-ending formatting by git

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63122/#review188572 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 18,

Review Request 63122: Remove corrupted fonts and add font files to .gitattributes to prevent line-ending formatting by git

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63122/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188569 --- Ship it! Master (b7e83a0) is green with this patch.

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Bill Farner
> On Oct. 17, 2017, 11:26 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 67-68 (patched) > > > > > > Since the Scheduler fails over every 24

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/#review188567 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 18,

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread David McLaughlin
> On Oct. 17, 2017, 6:26 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 67-68 (patched) > > > > > > Since the Scheduler fails over every 24

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62956/ --- (Updated Oct. 18, 2017, 1:06 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 63117: Role and Environment Page fixes

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63117/ --- (Updated Oct. 18, 2017, 8:04 p.m.) Review request for Aurora, Kai Huang and

Re: Review Request 63117: Role and Environment Page fixes

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63117/ --- (Updated Oct. 18, 2017, 8:02 p.m.) Review request for Aurora, Kai Huang and

Review Request 63117: Instance Page fixes

2017-10-18 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63117/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Bill Farner
> On Oct. 17, 2017, 11:26 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 67-68 (patched) > > > > > > Since the Scheduler fails over every 24

Re: Review Request 62956: Immediately reject offers lacking necessary resources

2017-10-18 Thread Bill Farner
> On Oct. 13, 2017, 1:12 a.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Lines 67-68 (patched) > > > > > > As far as I know this will filter this agent entirely for

Re: Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63121/#review188558 --- Master (b7e83a0) is red with this patch.

Review Request 63121: Remove static bans for task groups that are no longer pending

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63121/ --- Review request for Aurora and Jordan Ly. Repository: aurora Description

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-10-18 Thread Nicolás Donatucci
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60942/#review188555 --- I found an issue a while back after talking to Joshua. It is

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

2017-10-18 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188544 --- Ship it! Thanks. You have answered all my questions and the

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

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188542 --- This patch does not apply cleanly against master (b7e83a0), do

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

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/ --- (Updated Oct. 18, 2017, 11:11 a.m.) Review request for Aurora, Jordan Ly and

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

2017-10-18 Thread Bill Farner
> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java > > Lines 79-103 (original), 72-96 (patched) > > > > > > Should these be

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

2017-10-18 Thread Bill Farner
> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote: > > Overall LGTM as well! I believe the logic for upgrading and maintaining > > forwards/backwards compatibility is sound. Ensuring my understanding, the > > upgrade path is: > > > > 1. A new version with the memory stores is released. > > 2.

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

2017-10-18 Thread Bill Farner
> On Oct. 12, 2017, 2:53 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java > > Lines 190-196 (patched) > > > > > > You can combine these, right? > >

Re: Review Request 63098: Clean up Job Page CSS

2017-10-18 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63098/#review188524 --- Ship it! Ship It! - Kai Huang On Oct. 18, 2017, 3:29 a.m.,

Re: Review Request 63092: Detect and parse Thermos config in Diff output

2017-10-18 Thread David McLaughlin
> On Oct. 18, 2017, 5:14 p.m., Santhosh Kumar Shanmugham wrote: > > Can we pretty-print the configs? (Removing all the escaped quotes and all) The only quotes left will be in strings (like cmdline) that contain other quoted strings. - David

Re: Review Request 63092: Detect and parse Thermos config in Diff output

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63092/#review188521 --- Ship it! Can we pretty-print the configs? (Removing all the

Re: Review Request 63098: Clean up Job Page CSS

2017-10-18 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63098/#review188519 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 17,

Re: Review Request 63099: Add Source Sans Pro font to project

2017-10-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63099/#review188515 --- Ship it! Ship It! - Jordan Ly On Oct. 18, 2017, 4:19 a.m.,

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

2017-10-18 Thread Bill Farner
> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java > > Lines 43-52 (patched) > > > > > > With multiple accesses of `hostAttributes` this