> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/Jobs.java, line 53
> > <https://reviews.apache.org/r/20058/diff/1/?file=549768#file549768line53>
> >
> >     A comment here about why SANDBOX_DELETED case is absent would be nice 
> > to tie in the context from above.

Sure. Added.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java, line 103
> > <https://reviews.apache.org/r/20058/diff/1/?file=549770#file549770line103>
> >
> >     Drop the TODO in favor of a NOTE, since we will likely not fix this 
> > behavior here.

Done.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java, line 
> > 107
> > <https://reviews.apache.org/r/20058/diff/1/?file=549775#file549775line107>
> >
> >     s/it/this/

Done.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java, line 
> > 456
> > <https://reviews.apache.org/r/20058/diff/1/?file=549775#file549775line456>
> >
> >     Can you leave a TODO to re-evaluate this?  My gut tells me *DELETED 
> > should not be valid transitions here.

Done.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java, line 
> > 534
> > <https://reviews.apache.org/r/20058/diff/1/?file=549775#file549775line534>
> >
> >     better doc is needed to explain what an absent status means
> >     
> >     a TODO to formalize the delete/absent with a delete() method would be 
> > really nice

Added both.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 263
> > <https://reviews.apache.org/r/20058/diff/1/?file=549777#file549777line263>
> >
> >     Is this needed?  the values have not changed, only a name, so 
> > previously-bogus values are still bogus.

This is to compensate for an increased likelihood of showing a stack trace due 
to adding SANDBOX_DELETED into the list of terminal states.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java, line 60
> > <https://reviews.apache.org/r/20058/diff/1/?file=549784#file549784line60>
> >
> >     This is a strange function (it was strange before, too), since only one 
> > status is provided.  This signature is pretty clear, though:
> >     
> >       List<TaskEvent> makeTaskEvents(
> >           long startTs,
> >           ScheduleStatus status,
> >           ScheduleStatus... statuses)
> >     
> >     Can you try that out?

Sure, refactored.


> On April 5, 2014, 11:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java, 
> > line 132
> > <https://reviews.apache.org/r/20058/diff/1/?file=549788#file549788line132>
> >
> >     Is this redundant to the exhaustive state transition test case?  
> > Possibly ditto for other new test cases in this file.

The majority of scenario-driven tests in this file can be replaced by the 
exhaustive test without losing on coverage. Yet I still think we need them all 
as they assert specific task lifetime stories rather than the possible 
transition matrix.


- Maxim


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


On April 5, 2014, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20058/
> -----------------------------------------------------------
> 
> (Updated April 5, 2014, 1:12 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-261
>     https://issues.apache.org/jira/browse/AURORA-261
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Rolling in the SANDBOX_DELETED changes originally introduced in 
> https://reviews.apache.org/r/19648/. 
> Also dropping StorageBackfill rewriting of UNKNOWN.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 
> 772511f74fc12cf320f9eb4dcc857c58a3546aa1 
>   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
> 4ac218b8f25be5e9e6b6243558af4258efe6e61a 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 003d475a1bd4ecc099d9a641fd239a8189f71cdb 
>   src/main/java/org/apache/aurora/scheduler/http/SchedulerzJob.java 
> 2ccc6f367b9715a0abb3e0673069289ae4860087 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java 
> 8e3bfd318309918d92cfb75312fefb025a0002e4 
>   src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 
> 2f6800043839dbb586bc774ccccb13fdd16ee09c 
>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 
> 4249707b283da078321faaaed006de54519238bc 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> 0161fac838ff5166161acd2960058de6856b0e9c 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> d2becea60e5d7bb59a2e5adb66e10cd50f6b56f3 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 9abab00bddd48e7495eb7fdb60d71ea05e2a862a 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 4a866b2d6041a592692812eb3472db744d21e194 
>   src/main/resources/org/apache/aurora/scheduler/http/schedulerzjob.st 
> 28b56671b2e825912a6427e609c2bbe1e7758e26 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> c0618e4edebd6f282698abfd9bdc3c36fff16920 
>   src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java 
> 534b2b62cc2ae0ae9baab0247c9679350f03042b 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> a4e9464f7d5d3f5a640b62557c3e29f2f1566985 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 378ece964081a7cb475dc6edd9b19ed506f03ec8 
>   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
> effe14bf36d967b64a30af77378f106b862944cd 
>   src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 2006ddb7f83a8ad2a1ce6ff3a96faf3c0bd8d8e9 
>   src/test/java/org/apache/aurora/scheduler/base/TasksTest.java 
> 102fe04f8d3a7142a0cd58251e16d31e8d4a433d 
>   
> src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
>  f834bfb8c2339585214b0512e7df98dc75f931c7 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
> 24949c043b16a35a7960ed0a40b79f0981179eae 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> e77063a9c8e40e015ec264b151a7ed76f1c7f00f 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> c6e318cabde9aad801b2a546415808473c9aff86 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> 05c6e8a1e2407f2822b3844c555d8995f1cd1d49 
> 
> Diff: https://reviews.apache.org/r/20058/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh 
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to