> On Nov. 5, 2014, 12:43 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java, line 
> > 318
> > <https://reviews.apache.org/r/27598/diff/1/?file=749747#file749747line318>
> >
> >     why not initialize this inline?
> 
> Maxim Khutornenko wrote:
>     Not sure I get it. Don't we need a handle to pull these mappings into 
> test?
> 
> Kevin Sweeney wrote:
>     I'm proposing making the field `@VisibleForTesting` directly and reading 
> it in the test. Slightly less code but either approach works.
> 
> Maxim Khutornenko wrote:
>     Ah, I see what you meant now. Inlining isn't going to work here as 
> closures use writeBehind- instance fields and I'd rather not move that much 
> code into the constructor.
> 
> Kevin Sweeney wrote:
>     writeBehind is an instance field so they can use it from there. I'm 
> proposing (simplified)
>     
>     ```java
>     class OurClass {
>       private final Storage storage;
>       @VisibleForTesting
>       final Map<Field, Closure<Entry>> HANDLERS = ImmutableMap.builder()
>         .add(
>           REMOVE_TASK,
>           (entry) -> storage.write(
>             (storeProvider) -> storeProvider.getTaskStore().removeTask(
>               entry.getRemoveTask().getTaskId())))
>          ...
>         .build();
>     }
>     ```

Right, it still does not pass checkstyle though: "Variable 
'logEntryReplayActions' must be private and have accessor methods."


- Maxim


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


On Nov. 5, 2014, 1:52 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27598/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 1:52 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-912
>     https://issues.apache.org/jira/browse/AURORA-912
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Fixing the log replay for the job update history pruner.
> 
> Also, refactored LogStorage replay routine to fully test all 
> LogEntry/LogEntry.TRANSACTION handlers.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> cbab75964052a950e1b868b3a53eb15fadb31cb7 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 8eb5c3f7c542206066b39a09911c7df01a43bee7 
> 
> Diff: https://reviews.apache.org/r/27598/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to