aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 38112: Alter thrift wrapper generator to use default primitive values and empty collections.
Date Tue, 08 Sep 2015 22:47:42 GMT

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

(Updated Sept. 8, 2015, 3:47 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
-------

Isolated change in DbJobUpdateStoreTest to affect fewer test cases.


Bugs: AURORA-1476
    https://issues.apache.org/jira/browse/AURORA-1476


Repository: aurora


Description (updated)
-------

Alter thrift wrapper generator to use default primitive values and empty collections.

Reviewers - please see my self-review commentary in specific parts of the patch.  The biggest
shift with this change is that an impedance mismatch that existed when inserting/fetching
a record from the database is now lifted to the thrift wrapper layer.

This means the following test is not guaranteed to pass:
```java
TaskConfig original = new TaskConfig(...);
assertEquals(original, ITaskConfig.build(original).newBuilder());  // won't always pass
```

Most specifically, the wrapped/upwrapped `TaskConfig` will have null collections replaced
with empty ones, and will not honor set/unset flags for primitives.  The best practice, therefore,
should be to treat our wrapper classes as the canonical form, and only prefer with the underlying
thrift types when absolutely necessary.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java a952797315a3421695748f09b9a6abb552cbb668

  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 8787aeaa6655cfab1e0a6d5719f9e08a89df7631

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java f1b167bacbfce4f753fc0bbb2b860e3024b9843f

  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java e12ad3e3868472ba84e379986bd1aa97bca42ffe

  src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py b5f2bc9e54b525a6a782d8873c9112f6496cd3f2

  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java b2ec13f40c12e5ee5663f4465734d6a80f3587cd

  src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java f3b62cc957186bc9673060830572bc1cc073ac49

  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 5d8bd1b72927786df95c972df64d68c78f25dad0

  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 08e1284ac1ef08b7649ed83df0a55be04cfeb88f

  src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java 9213b88ab4ce5063ca0fb055851ae5632616155e

  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java db60cd21d06d636505202bac7277a13dc24d46e6

  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 295974a9f97e020dce11474d500a1bcd40d9f5d5

  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 7ccc273204d20c84bbb576958e832b6f4a29f607

  src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java 0e0a847f46c4d1d833a3c610e8a5f752f368c0d5

  src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 3e78c097a7a9252ded8a4a7fc6609ecf5d61c5b5

  src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java e0110e7ebe631b7b66b2341cedc10490da00a2ab

  src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 4d4e752088f7dca99675cc66782ae046bbd516d6

  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 4685aa157be77502ad0e4e648ad333ee286f3de5


Diff: https://reviews.apache.org/r/38112/diff/


Testing
-------

End-to-end tests pass


Thanks,

Bill Farner


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message