----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63521/#review189940 -----------------------------------------------------------
Ship it! Ship It! - Bill Farner On Nov. 2, 2017, 11:12 a.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63521/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 11:12 a.m.) > > > Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner. > > > Repository: aurora > > > Description > ------- > > A bug was introduced when the old `MemAttributeStore` was revived. > Previously, the `saveHostAttributes` method did not return anything. However, > after migrating to the DB stores, the signature of the interface was changed > to return a `boolean` if the save modified the previous attributes. The new > changes accidentally inverted the order. The old `MemAttributeStoreTest` did > not explicity test for this scenario so it went unnoticed. > > The interface involved: > ``` > /** > * Save a host attribute in the attribute store. > * > * @param hostAttributes The attribute we are going to save. > * @return {@code true} if the operation changed the attributes stored for > the given > * {@link IHostAttributes#getHost() host}, or {@code false} if the > save was a no-op. > */ > boolean saveHostAttributes(IHostAttributes hostAttributes); > ``` > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java > 483af194787e967a97c908a62889233336407aba > > src/test/java/org/apache/aurora/scheduler/storage/AbstractAttributeStoreTest.java > 34db54be6eecbf0eaeab4fa2a19e6a66469cea88 > > > Diff: https://reviews.apache.org/r/63521/diff/1/ > > > Testing > ------- > > Fixed tests so they take into account the return value of the save. > Added a test explicity testing this behavior. > > > Thanks, > > Jordan Ly > >
