Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============2779691464163184620=="
MIME-Version: 1.0
Subject: Review Request 63521: Fixed issue where saving attributes are not
 being persisted to log
From: Jordan Ly <jordan.ly8@gmail.com>
To: Bill Farner <wfarner@apache.org>, David McLaughlin <david@dmclaughlin.com>,
 Stephan Erb <serb@apache.org>
Cc: Aurora <reviews@aurora.apache.org>, Jordan Ly <jordan.ly8@gmail.com>
Date: Thu, 02 Nov 2017 18:12:05 -0000
Message-ID: <20171102181205.55462.36988@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Jordan Ly <noreply@reviews.apache.org>
Reply-To: Jordan Ly <jordan.ly8@gmail.com>
archived-at: Thu, 02 Nov 2017 18:12:13 -0000

--===============2779691464163184620==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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

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


--===============2779691464163184620==--