Re: Review Request 63078: Hide pagination links on Role and Job lists when only one page

[Aurora ReviewBot]

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


Ship it!




Master (5b72398) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 10:06 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63078/
> ---
> 
> (Updated Oct. 17, 2017, 10:06 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Hide pagination links on Role and Job lists when only one page
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobList.js 
> ff5377de8ec0e91303003af6cb9f202e06f44889 
>   ui/src/main/js/components/RoleList.js 
> cffb01290d0f0413cd5f24013b609f4b5d4e9a6a 
> 
> 
> Diff: https://reviews.apache.org/r/63078/diff/1/
> 
> 
> Testing
> ---
> 
> Confirmed in Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63078: Hide pagination links on Role and Job lists when only one page

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Hide pagination links on Role and Job lists when only one page


Diffs
-

  ui/src/main/js/components/JobList.js ff5377de8ec0e91303003af6cb9f202e06f44889 
  ui/src/main/js/components/RoleList.js 
cffb01290d0f0413cd5f24013b609f4b5d4e9a6a 


Diff: https://reviews.apache.org/r/63078/diff/1/


Testing
---

Confirmed in Vagrant.


Thanks,

David McLaughlin

Review Request 63079: Center pagination links when not a table.

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Center pagination links when not a table.


Diffs
-

  ui/src/main/js/components/Pagination.js 
6a8b73ed0a9466e263c1647db97c2e3fce381c3f 
  ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
  ui/src/main/sass/components/_pagination.scss PRE-CREATION 


Diff: https://reviews.apache.org/r/63079/diff/1/


Testing
---


File Attachments


Updates Page fix
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/6f2323d9-05b7-4e55-b94b-b2429fb5e0de__Screen_Shot_2017-10-17_at_10.13.59_AM.png


Thanks,

David McLaughlin

Review Request 63082: Hide InstanceHistory when there are no old tasks.

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Hide InstanceHistory when no old tasks.


Diffs
-

  ui/src/main/js/components/InstanceHistory.js 
fb0639079d227ba19cf97ed1061d6a55db0f0497 
  ui/src/main/js/components/__tests__/InstanceHistory-test.js 
16314810312a48d44faeaa795640db48b225d243 


Diff: https://reviews.apache.org/r/63082/diff/1/


Testing
---


File Attachments


No old tasks
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/be3f31f6-d681-48e2-8ca7-708a85ca278b__Screen_Shot_2017-10-17_at_10.28.08_AM.png


Thanks,

David McLaughlin

Re: Review Request 63079: Center pagination links when not a table.

[Reza Motamedi]

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


Ship it!




Ship It!

- Reza Motamedi


On Oct. 17, 2017, 5:14 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63079/
> ---
> 
> (Updated Oct. 17, 2017, 5:14 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Center pagination links when not a table.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Pagination.js 
> 6a8b73ed0a9466e263c1647db97c2e3fce381c3f 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_pagination.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63079/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Updates Page fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/6f2323d9-05b7-4e55-b94b-b2429fb5e0de__Screen_Shot_2017-10-17_at_10.13.59_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63065: Fix link on Navigation logo

[Kai Huang]

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


Ship it!




Ship It!

- Kai Huang


On Oct. 17, 2017, 4:10 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63065/
> ---
> 
> (Updated Oct. 17, 2017, 4:10 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix link on Navigation logo
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Navigation.js 
> e46cafdaec08d24698b60434fb821f9933f130b9 
> 
> 
> Diff: https://reviews.apache.org/r/63065/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63081: Format constraints on Task Config Summary

[Reza Motamedi]

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


Ship it!




Ship It!

- Reza Motamedi


On Oct. 17, 2017, 5:21 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63081/
> ---
> 
> (Updated Oct. 17, 2017, 5:21 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Format constraints on Task Config Summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 69b1a40cc4419d5aecb3da7c8bcfb549b789dc7f 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
> 
> 
> Diff: https://reviews.apache.org/r/63081/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Constraint fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/020c1035-0e54-42ad-b685-17130e370e51__Screen_Shot_2017-10-17_at_10.20.52_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63078: Hide pagination links on Role and Job lists when only one page

[Reza Motamedi]

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


Ship it!




Ship It!

- Reza Motamedi


On Oct. 17, 2017, 5:06 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63078/
> ---
> 
> (Updated Oct. 17, 2017, 5:06 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Hide pagination links on Role and Job lists when only one page
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobList.js 
> ff5377de8ec0e91303003af6cb9f202e06f44889 
>   ui/src/main/js/components/RoleList.js 
> cffb01290d0f0413cd5f24013b609f4b5d4e9a6a 
> 
> 
> Diff: https://reviews.apache.org/r/63078/diff/1/
> 
> 
> Testing
> ---
> 
> Confirmed in Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63082: Hide InstanceHistory when there are no old tasks.

[David McLaughlin]

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

(Updated Oct. 17, 2017, 5:30 p.m.)


Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Changes
---

Delete stale code.


Repository: aurora


Description
---

Hide InstanceHistory when no old tasks.


Diffs (updated)
-

  ui/src/main/js/components/InstanceHistory.js 
fb0639079d227ba19cf97ed1061d6a55db0f0497 
  ui/src/main/js/components/__tests__/InstanceHistory-test.js 
16314810312a48d44faeaa795640db48b225d243 


Diff: https://reviews.apache.org/r/63082/diff/2/

Changes: https://reviews.apache.org/r/63082/diff/1-2/


Testing
---


File Attachments


No old tasks
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/be3f31f6-d681-48e2-8ca7-708a85ca278b__Screen_Shot_2017-10-17_at_10.28.08_AM.png


Thanks,

David McLaughlin

Review Request 63081: Format constraints on Task Config Summary

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Format constraints on Task Config Summary


Diffs
-

  ui/src/main/js/components/TaskConfigSummary.js 
69b1a40cc4419d5aecb3da7c8bcfb549b789dc7f 
  ui/src/main/sass/components/_job-page.scss 
8523fcf4c917b46c9d514325f073bd5788798156 


Diff: https://reviews.apache.org/r/63081/diff/1/


Testing
---


File Attachments


Constraint fix
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/020c1035-0e54-42ad-b685-17130e370e51__Screen_Shot_2017-10-17_at_10.20.52_AM.png


Thanks,

David McLaughlin

Re: Review Request 63062: Expose list of neighbors in the instance page

[Kai Huang]

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


Fix it, then Ship it!




LGTM


ui/src/main/sass/components/_instance-page.scss
Lines 31 (patched)


s/taskkey/task-key/
or task-instance-id


- Kai Huang


On Oct. 17, 2017, 5:34 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 17, 2017, 5:34 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/3/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63081: Format constraints on Task Config Summary

[Kai Huang]

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


Ship it!




Ship It!

- Kai Huang


On Oct. 17, 2017, 5:21 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63081/
> ---
> 
> (Updated Oct. 17, 2017, 5:21 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Format constraints on Task Config Summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 69b1a40cc4419d5aecb3da7c8bcfb549b789dc7f 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
> 
> 
> Diff: https://reviews.apache.org/r/63081/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Constraint fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/020c1035-0e54-42ad-b685-17130e370e51__Screen_Shot_2017-10-17_at_10.20.52_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

[Santhosh Kumar Shanmugham]

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



Mostly looks good to me. Some questions about concurrency when the stores go 
into action. We need a multi-threaded stress/load test.


src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
Lines 79-103 (original), 72-96 (patched)


Should these be removed? The release notes indicate these have been 
removed? Are we keeping this for the deprecation cycle?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 34 (patched)


public?

s/Mutable/AttributeStore.Mutable/



src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
Lines 43-52 (patched)


I guess you mean `replace` on `ConcurrentMap`?

+1 (Sounds more robust)



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 156-159 (patched)


Should we insert the refreshed entry into the map as soon as possible? 
Inserting all at once towards the end seems like it can cause races.



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 236 (patched)


`LockStore` can change after `tokenFromLockStore` returns. Should we 
protect against this race?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 298 (patched)


Same here- wonder if we can move to using `Stream`s?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 307 (patched)


Is this correct? Should it be - 

`System.currentTimeMillis() - s.getState().getCreatedTimestampMs() > 
historyPruneThresholdMs` 

?



src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java
Lines 345-360 (patched)


+1



src/main/java/org/apache/aurora/scheduler/storage/mem/MemLockStore.java
Lines 33 (patched)


public



src/main/java/org/apache/aurora/scheduler/storage/mem/MemQuotaStore.java
Lines 27-28 (patched)


public



src/main/java/org/apache/aurora/scheduler/storage/mem/MemSchedulerStore.java
Lines 26 (patched)


public

(Not sure which one is the convention that you are going for. Make all 
`Store`s public or package-private.)



src/test/java/org/apache/aurora/scheduler/storage/db/AttributeStoreTest.java
Lines 19 (patched)


Is this going to be all the test or do you intend to add more later? (Here 
and for all the `MemStore`s)


- Santhosh Kumar Shanmugham


On Oct. 10, 2017, 12:35 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62869/
> ---
> 
> (Updated Oct. 10, 2017, 12:35 p.m.)
> 
> 
> Review request for Aurora, Jordan Ly and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch introduces map-based volatile stores, most of which were revived 
> from git history with minimal changes.  The DB storage system is now only 
> used in a temporary storage when replaying a snapshot containing the 
> `dbScript` field.
> 
> Please take special care to double-check my work in `SnapshotStoreImpl`, as 
> it must function for backwards compatibility with `Snapshot`s in the wild.  
> Another area of interest is the implementation of `MemJobUpdateStore`, which 
> must have nuanced behavior for compatibility with `DbJobUpdateStore`.  Most 
> of this stems from behind-the-scenes interaction with `LockStore` and use of 
> cascading deletes.
> 
> Note that this change removes the transactional nature of in-memory storage 
> operations as well as the `READ COMMITTED` transaction isolation previously 
> available to some stores (proven in necessary changes to 
> `StorageTransactionTest`).  This means some stores will permit dirty reads 
> when they previously did not.  `TaskStore` has always had this 
> non-transactional behavior by default, as the DB task store was never deemed 
> suitable for production.  Nonetheless, this non-transactional behavior should 
> be considered safe as the scheduler fails over on a storage operation 
> failure, and relies on the persistent log storage for 

Re: Review Request 63081: Format constraints on Task Config Summary

[Aurora ReviewBot]

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


Ship it!




Master (5b72398) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 5:21 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63081/
> ---
> 
> (Updated Oct. 17, 2017, 5:21 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Format constraints on Task Config Summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 69b1a40cc4419d5aecb3da7c8bcfb549b789dc7f 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
> 
> 
> Diff: https://reviews.apache.org/r/63081/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Constraint fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/020c1035-0e54-42ad-b685-17130e370e51__Screen_Shot_2017-10-17_at_10.20.52_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63079: Center pagination links when not a table.

[Aurora ReviewBot]

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


Ship it!




Master (5b72398) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 10:14 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63079/
> ---
> 
> (Updated Oct. 17, 2017, 10:14 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Center pagination links when not a table.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Pagination.js 
> 6a8b73ed0a9466e263c1647db97c2e3fce381c3f 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_pagination.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63079/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Updates Page fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/6f2323d9-05b7-4e55-b94b-b2429fb5e0de__Screen_Shot_2017-10-17_at_10.13.59_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 62956: Immediately reject offers lacking necessary resources

[Santhosh Kumar Shanmugham]

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




src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
Lines 67-68 (patched)


Since the Scheduler fails over every 24 hours, maybe we can let the new 
Scheduler retry the Slave?

30 days seems like a very high threshold and can sneak into a tight 
capacity situation without much warning. Typically in those scenarios, we 
manually churn the cluster to free up space. Wonder how the 30 day filter would 
behave in such a case. Having said that, we should make this configurable with 
a resonable default (few hrs)?


- Santhosh Kumar Shanmugham


On Oct. 12, 2017, 4:18 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62956/
> ---
> 
> (Updated Oct. 12, 2017, 4:18 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There's no reason for us to evaluate offers with no CPUs or memory, so reject 
> them early in the offer lifecycle.
> 
> This is an incremental performance optimization, but it may net significant 
> improvements based on observations in some very large clusters.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 3c77e2983ce00f897f3d5ed106b779cd7f7f0940 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  1b1239753f40d7d46d91724def6c25037eb79f1c 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java 
> d5db81b88a0369d0b26c8fbf70efab3886ad7695 
>   src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java 
> b98aaaf48ae60afef19a368ee96abc897300f8fa 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/offers/Offers.java 
> 129b4437315c6ad4ea47ca75d4ae6e28cadd7911 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 765a527acb96997989c920be8b69dfa1113dc302 
> 
> 
> Diff: https://reviews.apache.org/r/62956/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63082: Hide InstanceHistory when there are no old tasks.

[Kai Huang]

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


Ship it!




Ship It!

- Kai Huang


On Oct. 17, 2017, 5:30 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63082/
> ---
> 
> (Updated Oct. 17, 2017, 5:30 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Hide InstanceHistory when no old tasks.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/InstanceHistory.js 
> fb0639079d227ba19cf97ed1061d6a55db0f0497 
>   ui/src/main/js/components/__tests__/InstanceHistory-test.js 
> 16314810312a48d44faeaa795640db48b225d243 
> 
> 
> Diff: https://reviews.apache.org/r/63082/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> No old tasks
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/be3f31f6-d681-48e2-8ca7-708a85ca278b__Screen_Shot_2017-10-17_at_10.28.08_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63082: Hide InstanceHistory when there are no old tasks.

[Aurora ReviewBot]

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


Ship it!




Master (b1648aa) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 5:30 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63082/
> ---
> 
> (Updated Oct. 17, 2017, 5:30 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Hide InstanceHistory when no old tasks.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/InstanceHistory.js 
> fb0639079d227ba19cf97ed1061d6a55db0f0497 
>   ui/src/main/js/components/__tests__/InstanceHistory-test.js 
> 16314810312a48d44faeaa795640db48b225d243 
> 
> 
> Diff: https://reviews.apache.org/r/63082/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> No old tasks
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/be3f31f6-d681-48e2-8ca7-708a85ca278b__Screen_Shot_2017-10-17_at_10.28.08_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63079: Center pagination links when not a table.

[Kai Huang]

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


Ship it!




Ship It!

- Kai Huang


On Oct. 17, 2017, 5:14 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63079/
> ---
> 
> (Updated Oct. 17, 2017, 5:14 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Center pagination links when not a table.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Pagination.js 
> 6a8b73ed0a9466e263c1647db97c2e3fce381c3f 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_pagination.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63079/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Updates Page fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/6f2323d9-05b7-4e55-b94b-b2429fb5e0de__Screen_Shot_2017-10-17_at_10.13.59_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[David McLaughlin]

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


Ship it!




LGTM with comments addressed.


File Attachment: Screen Shot 2017-10-16 at 6.27.44 PM.png - Screen Shot 
2017-10-16 at 6.27.44 PM.png


I'd also try and find a task with a longer job key and see what it does to 
the layout. It might make more sense to have this widget as a row spanning the 
whole width.


- David McLaughlin


On Oct. 17, 2017, 5:34 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 17, 2017, 5:34 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/3/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Review Request 63087: Fix instance range display

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Fix instance range display


Diffs
-

  ui/src/main/js/utils/Task.js 3259623091957ea39500e6b1caa79ac4b8a3d78d 
  ui/src/main/js/utils/__tests__/Task-test.js PRE-CREATION 


Diff: https://reviews.apache.org/r/63087/diff/1/


Testing
---

./gradlew ui:test
./gradlew ui:lint

See screenshots.


File Attachments


Screen Shot 2017-10-17 at 1.24.03 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/88a998ef-6cd1-4620-bfd8-85ebf792984a__Screen_Shot_2017-10-17_at_1.24.03_PM.png


Thanks,

David McLaughlin

Re: Review Request 63083: Add diff viewer to Update Page

[Aurora ReviewBot]

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


Ship it!




Master (94999eb) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 8:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63081: Format constraints on Task Config Summary

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 10:21 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63081/
> ---
> 
> (Updated Oct. 17, 2017, 10:21 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Format constraints on Task Config Summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 69b1a40cc4419d5aecb3da7c8bcfb549b789dc7f 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
> 
> 
> Diff: https://reviews.apache.org/r/63081/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Constraint fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/020c1035-0e54-42ad-b685-17130e370e51__Screen_Shot_2017-10-17_at_10.20.52_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63062: Expose list of neighbors in the instance page

[Santhosh Kumar Shanmugham]

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


Ship it!




LGTM. Improvement suggested.


File Attachment: Screen Shot 2017-10-16 at 6.27.44 PM.png - Screen Shot 
2017-10-16 at 6.27.44 PM.png


Wonder if we need to paginate this? Think the most neighbors I have seen is 
~20.



ui/src/main/js/components/TaskNeighbors.js
Lines 21 (patched)


`isNullyOrEmpty` already checks if the collection is empty? In which case 
`tasks.length === 0` is redudant.


- Santhosh Kumar Shanmugham


On Oct. 16, 2017, 10:34 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63062/
> ---
> 
> (Updated Oct. 16, 2017, 10:34 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kai Huang, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Expose list of neighbors in the instance page
> 
> Exposing the list of neighbor task on the host is useful when application 
> tail letencies degrade because of noisy neighbors. Currenlty our service 
> oweners need to jump to the thermos-UI to get this info. This feature will 
> definately help spotting misbehaving neighbors a lot easier.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskNeighbors.js PRE-CREATION 
>   ui/src/main/js/components/TaskStatus.js 
> b5149182c80b0edcdec88d5c6e24d3015409b0fc 
>   ui/src/main/js/components/__tests__/TaskNeighbors-test.js PRE-CREATION 
>   ui/src/main/js/pages/Instance.js c4d625c063b6037ae2ff93e638a3040436515cd5 
>   ui/src/main/js/utils/Common.js 8f2da7c8a5b44d2ded89a2070ea6ac4bf1fb6ed5 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
> 
> 
> Diff: https://reviews.apache.org/r/63062/diff/3/
> 
> 
> Testing
> ---
> 
> ? ./gradlew ui:test
> ? ./gradlew ui:lint
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-16 at 6.27.44 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/7fa71abf-f83e-412f-b554-dc880ccdb25c__Screen_Shot_2017-10-16_at_6.27.44_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 62956: Immediately reject offers lacking necessary resources

[Jordan Ly]

> On Oct. 17, 2017, 6:26 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> > Lines 67-68 (patched)
> > 
> >
> > Since the Scheduler fails over every 24 hours, maybe we can let the new 
> > Scheduler retry the Slave?
> > 
> > 30 days seems like a very high threshold and can sneak into a tight 
> > capacity situation without much warning. Typically in those scenarios, we 
> > manually churn the cluster to free up space. Wonder how the 30 day filter 
> > would behave in such a case. Having said that, we should make this 
> > configurable with a resonable default (few hrs)?

I believe that the filter only works against a specific framework ID, so that a 
scheduler failover or deploy would receive the offers again.


- Jordan


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


On Oct. 12, 2017, 11:18 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62956/
> ---
> 
> (Updated Oct. 12, 2017, 11:18 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There's no reason for us to evaluate offers with no CPUs or memory, so reject 
> them early in the offer lifecycle.
> 
> This is an incremental performance optimization, but it may net significant 
> improvements based on observations in some very large clusters.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 3c77e2983ce00f897f3d5ed106b779cd7f7f0940 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  1b1239753f40d7d46d91724def6c25037eb79f1c 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java 
> d5db81b88a0369d0b26c8fbf70efab3886ad7695 
>   src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java 
> b98aaaf48ae60afef19a368ee96abc897300f8fa 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/offers/Offers.java 
> 129b4437315c6ad4ea47ca75d4ae6e28cadd7911 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 765a527acb96997989c920be8b69dfa1113dc302 
> 
> 
> Diff: https://reviews.apache.org/r/62956/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63064: Clean up State Machine CSS to handle long messages

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 16, 2017, 9:06 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63064/
> ---
> 
> (Updated Oct. 16, 2017, 9:06 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Clean up State Machine CSS to handle long messages
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/StateMachine.js 
> 2da85f77d4f0a34adc1ddb291482afa1ea8e51f1 
>   ui/src/main/sass/components/_layout.scss 
> 4818ab26006af5ac9e54189bf9c88d771f9e9391 
>   ui/src/main/sass/components/_state-machine.scss 
> 804746d10a6972c3ec6eb09539b97f33ee20e15d 
> 
> 
> Diff: https://reviews.apache.org/r/63064/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Current
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/442c4ffa-f0d7-494b-8b54-e8cf32141a75__Screen_Shot_2017-10-16_at_8.54.47_PM.png
> Fixed
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/42b3ca44-2c09-4da9-9165-1a4f12745b46__Screen_Shot_2017-10-16_at_8.59.27_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63079: Center pagination links when not a table.

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 10:14 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63079/
> ---
> 
> (Updated Oct. 17, 2017, 10:14 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Center pagination links when not a table.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Pagination.js 
> 6a8b73ed0a9466e263c1647db97c2e3fce381c3f 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_pagination.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63079/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Updates Page fix
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/6f2323d9-05b7-4e55-b94b-b2429fb5e0de__Screen_Shot_2017-10-17_at_10.13.59_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63083: Add diff viewer to Update Page

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Add diff viewer from Job Page to Update Page. Key difference on update page is 
that the "after" diff is fixed - it's always the desiredState. But I've 
extracted the Diff rendering into a shared component between ConfigDiff and 
UpdateDiff, as well as the CSS.


Diffs
-

  ui/src/main/js/components/ConfigDiff.js 
9627751293ff01c9a3b728e2c352894f1a1e22f3 
  ui/src/main/js/components/Diff.js PRE-CREATION 
  ui/src/main/js/components/InstanceViz.js 
99efec4c15fcc677820b3145f506997ef8a37207 
  ui/src/main/js/components/UpdateConfig.js 
fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
  ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
  ui/src/main/js/components/__tests__/ConfigDiff-test.js 
3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
  ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js 
2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
  ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
  ui/src/main/sass/components/_diff.scss PRE-CREATION 
  ui/src/main/sass/components/_job-page.scss 
8523fcf4c917b46c9d514325f073bd5788798156 
  ui/src/main/sass/components/_update-page.scss 
a0db0b43b61e8ef10c29195945e7543a982aeab1 


Diff: https://reviews.apache.org/r/63083/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Selecting from multiple initial configs
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
Showing diff by default
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png


Thanks,

David McLaughlin

Re: Review Request 62958: Add URL handling for tab switching on Job page

[David McLaughlin]

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



Due to the lack of strong opinions - I'm going to ship this as-is. I think it's 
worth keeping the flexibility to have URL state across multiple components on 
the job page. If anyone feels strongly, feel free to post a patch to change.

- David McLaughlin


On Oct. 13, 2017, 12:10 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62958/
> ---
> 
> (Updated Oct. 13, 2017, 12:10 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This maintains the existing UI functionality where clicking on tabs updates 
> the browser location, to create shareable URLs to tab views.
> 
> 
> Diffs
> -
> 
>   ui/package.json cde8d106346d9ac498cfee9b5291ebc637fc6a2a 
>   ui/src/main/js/components/Tabs.js 43b1950b11b80dc3017730801992341bc527c39c 
>   ui/src/main/js/components/__tests__/Tabs-test.js 
> e028c2dd86739ed9762aa1d0be5c609d5487e06e 
>   ui/src/main/js/pages/Job.js fc400f7442a1f8a5f0ebfe366dfe40ef40e7108e 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 4cc76b8731d71ceca87a5d1e259360b2af8feba0 
> 
> 
> Diff: https://reviews.apache.org/r/62958/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> Unfortunately the testing here is only on one side (restoring the URL state). 
> I cannot simulate the change events from the unit tests because of 
> limitations with shallow rendering. When I figure out how to add integration 
> tests that work with React Router, I'll add coverage.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63087: Fix instance range display

[David McLaughlin]

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

(Updated Oct. 17, 2017, 9:43 p.m.)


Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Changes
---

Fix lint issue.


Repository: aurora


Description
---

Fix instance range display


Diffs (updated)
-

  ui/src/main/js/utils/Task.js 3259623091957ea39500e6b1caa79ac4b8a3d78d 
  ui/src/main/js/utils/__tests__/Task-test.js PRE-CREATION 


Diff: https://reviews.apache.org/r/63087/diff/2/

Changes: https://reviews.apache.org/r/63087/diff/1-2/


Testing
---

./gradlew ui:test
./gradlew ui:lint

See screenshots.


File Attachments


Screen Shot 2017-10-17 at 1.24.03 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/88a998ef-6cd1-4620-bfd8-85ebf792984a__Screen_Shot_2017-10-17_at_1.24.03_PM.png


Thanks,

David McLaughlin

Re: Review Request 63083: Add diff viewer to Update Page

[David McLaughlin]

> On Oct. 17, 2017, 9:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
> Currently that would have to assume the executor config is JSON.
> 
> David McLaughlin wrote:
> But my long term plan is to look for "AuroraExecutor" in the config and 
> do nice things for Thermos (and allow other people to drop in their own 
> parsers for custom executors).
> 
> Stephan Erb wrote:
> That is why I'd propose to do it here: 
> https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/config/thrift.py#L328
> 
> At that point in time we are sure this is a Thermos config in json. As it 
> is just a blob for Aurora, formatting it should not really matter.
> 
> Stephan Erb wrote:
> I agree thought hat a specific `"AuroraExecutor"` mode would be the best 
> thing. My proposal is more like a very easy workaround.

Even if it's pretty printed, the issue is that because it's a string the entire 
string is considered a diff and you will just see two huge red/green blobs in 
the output. What we want is to deserialize the JSON and only see the parts 
which have actually change. All we need is the confidence to do JSON.parse on 
the executor data.


- David


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


On Oct. 17, 2017, 8:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[Stephan Erb]

> On Oct. 17, 2017, 11:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
> Currently that would have to assume the executor config is JSON.
> 
> David McLaughlin wrote:
> But my long term plan is to look for "AuroraExecutor" in the config and 
> do nice things for Thermos (and allow other people to drop in their own 
> parsers for custom executors).
> 
> Stephan Erb wrote:
> That is why I'd propose to do it here: 
> https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/config/thrift.py#L328
> 
> At that point in time we are sure this is a Thermos config in json. As it 
> is just a blob for Aurora, formatting it should not really matter.
> 
> Stephan Erb wrote:
> I agree thought hat a specific `"AuroraExecutor"` mode would be the best 
> thing. My proposal is more like a very easy workaround.
> 
> David McLaughlin wrote:
> Even if it's pretty printed, the issue is that because it's a string the 
> entire string is considered a diff and you will just see two huge red/green 
> blobs in the output. What we want is to deserialize the JSON and only see the 
> parts which have actually change. All we need is the confidence to do 
> JSON.parse on the executor data.

Ah alright. Thanks for clarifying.


- Stephan


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


On Oct. 17, 2017, 11:57 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 11:57 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss c4b14135b4721327436db14159387ea0ea335a41 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> cd44e36df37aecf650f7df4c9f2d27ce9121b9ce 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63091: Dummy

[Santhosh Kumar Shanmugham]

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

Review request for Aurora.


Repository: aurora


Description
---

Dummy


Diffs
-

  RELEASE-NOTES.md 079f495d2f41272456daec5caca0944aa2d7fafc 


Diff: https://reviews.apache.org/r/63091/diff/1/


Testing
---

Just testing review board.


Thanks,

Santhosh Kumar Shanmugham

Re: Review Request 62956: Immediately reject offers lacking necessary resources

[Jordan Ly]

> On Oct. 17, 2017, 6:26 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> > Lines 67-68 (patched)
> > 
> >
> > Since the Scheduler fails over every 24 hours, maybe we can let the new 
> > Scheduler retry the Slave?
> > 
> > 30 days seems like a very high threshold and can sneak into a tight 
> > capacity situation without much warning. Typically in those scenarios, we 
> > manually churn the cluster to free up space. Wonder how the 30 day filter 
> > would behave in such a case. Having said that, we should make this 
> > configurable with a resonable default (few hrs)?
> 
> Jordan Ly wrote:
> I believe that the filter only works against a specific framework ID, so 
> that a scheduler failover or deploy would receive the offers again.
> 
> Jordan Ly wrote:
> Additionally, does churning the cluster mean new offers would be 
> generated? If so, I think that they would get a new offer ID and be reissued.
> 
> Stephan Erb wrote:
> The framework ID remains constent across failovers of both Aurora 
> schedulers and Mesos masters. Otherwise we'd lose all currently runnings 
> tasks during a failover.
> 
> For the filtering I am under the impression that it is per agent and 
> independent of offer or offer IDs. To be safe, we should check with some 
> Mesos developers though :)

You are correct, the framework ID will remain constant and the filters will 
stay in place.

For the filtering, I am being told that if you refuse an offer with x 
resources, then if those resources stay the same Mesos will not offer them to 
you again. However, if the resources increases then Mesos will offer them to 
the framework again.

Could we take advantage of the reviveOffers() call to remove filters on 
scheduler initialization?


- Jordan


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


On Oct. 12, 2017, 11:18 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62956/
> ---
> 
> (Updated Oct. 12, 2017, 11:18 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There's no reason for us to evaluate offers with no CPUs or memory, so reject 
> them early in the offer lifecycle.
> 
> This is an incremental performance optimization, but it may net significant 
> improvements based on observations in some very large clusters.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 3c77e2983ce00f897f3d5ed106b779cd7f7f0940 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  1b1239753f40d7d46d91724def6c25037eb79f1c 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java 
> d5db81b88a0369d0b26c8fbf70efab3886ad7695 
>   src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java 
> b98aaaf48ae60afef19a368ee96abc897300f8fa 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/offers/Offers.java 
> 129b4437315c6ad4ea47ca75d4ae6e28cadd7911 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 765a527acb96997989c920be8b69dfa1113dc302 
> 
> 
> Diff: https://reviews.apache.org/r/62956/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63082: Hide InstanceHistory when there are no old tasks.

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 10:30 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63082/
> ---
> 
> (Updated Oct. 17, 2017, 10:30 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Hide InstanceHistory when no old tasks.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/InstanceHistory.js 
> fb0639079d227ba19cf97ed1061d6a55db0f0497 
>   ui/src/main/js/components/__tests__/InstanceHistory-test.js 
> 16314810312a48d44faeaa795640db48b225d243 
> 
> 
> Diff: https://reviews.apache.org/r/63082/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> No old tasks
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/be3f31f6-d681-48e2-8ca7-708a85ca278b__Screen_Shot_2017-10-17_at_10.28.08_AM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63088: Add pointer for pagination links

[Aurora ReviewBot]

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


Ship it!




Master (e161c97) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 8:39 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63088/
> ---
> 
> (Updated Oct. 17, 2017, 8:39 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add pointer for pagination links
> 
> 
> Diffs
> -
> 
>   ui/src/main/sass/components/_pagination.scss 
> 337249145948e73b13f6d9b2a44936ca4490348b 
> 
> 
> Diff: https://reviews.apache.org/r/63088/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[Stephan Erb]

> On Oct. 17, 2017, 11:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
> Currently that would have to assume the executor config is JSON.
> 
> David McLaughlin wrote:
> But my long term plan is to look for "AuroraExecutor" in the config and 
> do nice things for Thermos (and allow other people to drop in their own 
> parsers for custom executors).

That is why I'd propose to do it here: 
https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/config/thrift.py#L328

At that point in time we are sure this is a Thermos config in json. As it is 
just a blob for Aurora, formatting it should not really matter.


- Stephan


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


On Oct. 17, 2017, 10:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 10:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63087: Fix instance range display

[Aurora ReviewBot]

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



Master (e161c97) is red with this patch.
  ./build-support/jenkins/build.sh

  1 error, 0 warnings potentially fixable with the `--fix` option.


npm ERR! Linux 4.4.0-43-generic
npm ERR! argv 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/node"
 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/npm"
 "run" "lint"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code ELIFECYCLE
npm ERR! apache-aurora@1.0.0 lint: `eslint src/main/js --ext .js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the apache-aurora@1.0.0 lint script 'eslint src/main/js 
--ext .js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the apache-aurora 
package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! eslint src/main/js --ext .js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs apache-aurora
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls apache-aurora
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR! /home/jenkins/jenkins-slave/workspace/AuroraBot/ui/npm-debug.log
:ui:lint FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':ui:lint'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/ui/.gradle/nodejs/node-v6.9.1-linux-x64/bin/npm''
>  finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 5m 37s
57 actionable tasks: 46 executed, 11 up-to-date


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 9:43 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63087/
> ---
> 
> (Updated Oct. 17, 2017, 9:43 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix instance range display
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/utils/Task.js 3259623091957ea39500e6b1caa79ac4b8a3d78d 
>   ui/src/main/js/utils/__tests__/Task-test.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63087/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:test
> ./gradlew ui:lint
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-17 at 1.24.03 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/88a998ef-6cd1-4620-bfd8-85ebf792984a__Screen_Shot_2017-10-17_at_1.24.03_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63092: Detect and parse Thermos config in Diff output

[David McLaughlin]

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

(Updated Oct. 17, 2017, 11:19 p.m.)


Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
Erb.


Changes
---

Explain the change.


Repository: aurora


Description (updated)
---

Add support for Thermos configs in Diff viewer. This deserializes the opaque 
"data" string so that the jsonDiff algorithm creates finer grained diffs.


Diffs
-

  ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
  ui/src/main/js/components/__tests__/Diff-test.js 
7f913223a3aa8ae6ce5322936e2653727481d549 
  ui/src/main/js/test-utils/TaskBuilders.js 
8427722056917d5e3a19d64ca36bc65aa6b5b85b 
  ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
  ui/src/main/sass/components/_diff.scss 
f58f7bde33e123a50ade0026a35d880593c4274b 


Diff: https://reviews.apache.org/r/63092/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Finer grained diff
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png


Thanks,

David McLaughlin

Re: Review Request 63083: Add diff viewer to Update Page

[Stephan Erb]

> On Oct. 17, 2017, 11:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
> Currently that would have to assume the executor config is JSON.
> 
> David McLaughlin wrote:
> But my long term plan is to look for "AuroraExecutor" in the config and 
> do nice things for Thermos (and allow other people to drop in their own 
> parsers for custom executors).
> 
> Stephan Erb wrote:
> That is why I'd propose to do it here: 
> https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/config/thrift.py#L328
> 
> At that point in time we are sure this is a Thermos config in json. As it 
> is just a blob for Aurora, formatting it should not really matter.

I agree thought hat a specific `"AuroraExecutor"` mode would be the best thing. 
My proposal is more like a very easy workaround.


- Stephan


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


On Oct. 17, 2017, 10:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 10:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[David McLaughlin]

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

(Updated Oct. 17, 2017, 9:57 p.m.)


Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Changes
---

Rebase before push.


Repository: aurora


Description
---

Add diff viewer from Job Page to Update Page. Key difference on update page is 
that the "after" diff is fixed - it's always the desiredState. But I've 
extracted the Diff rendering into a shared component between ConfigDiff and 
UpdateDiff, as well as the CSS.


Diffs (updated)
-

  ui/src/main/js/components/ConfigDiff.js 
9627751293ff01c9a3b728e2c352894f1a1e22f3 
  ui/src/main/js/components/Diff.js PRE-CREATION 
  ui/src/main/js/components/InstanceViz.js 
99efec4c15fcc677820b3145f506997ef8a37207 
  ui/src/main/js/components/UpdateConfig.js 
fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
  ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
  ui/src/main/js/components/__tests__/ConfigDiff-test.js 
3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
  ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js 
2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
  ui/src/main/sass/app.scss c4b14135b4721327436db14159387ea0ea335a41 
  ui/src/main/sass/components/_diff.scss PRE-CREATION 
  ui/src/main/sass/components/_job-page.scss 
cd44e36df37aecf650f7df4c9f2d27ce9121b9ce 
  ui/src/main/sass/components/_update-page.scss 
a0db0b43b61e8ef10c29195945e7543a982aeab1 


Diff: https://reviews.apache.org/r/63083/diff/2/

Changes: https://reviews.apache.org/r/63083/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Selecting from multiple initial configs
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
Showing diff by default
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png


Thanks,

David McLaughlin

Review Request 63092: Detect and parse Thermos config in Diff output

[David McLaughlin]

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

Review request for Aurora and Stephan Erb.


Repository: aurora


Description
---

Add support for Thermos configs in Diff viewer.


Diffs
-

  ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
  ui/src/main/js/components/__tests__/Diff-test.js 
7f913223a3aa8ae6ce5322936e2653727481d549 
  ui/src/main/js/test-utils/TaskBuilders.js 
8427722056917d5e3a19d64ca36bc65aa6b5b85b 
  ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
  ui/src/main/sass/components/_diff.scss 
f58f7bde33e123a50ade0026a35d880593c4274b 


Diff: https://reviews.apache.org/r/63092/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Finer grained diff
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png


Thanks,

David McLaughlin

Re: Review Request 63092: Detect and parse Thermos config in Diff output

[Aurora ReviewBot]

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


Ship it!




Master (0efe415) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 17, 2017, 10:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63092/
> ---
> 
> (Updated Oct. 17, 2017, 10:44 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for Thermos configs in Diff viewer.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
>   ui/src/main/js/components/__tests__/Diff-test.js 
> 7f913223a3aa8ae6ce5322936e2653727481d549 
>   ui/src/main/js/test-utils/TaskBuilders.js 
> 8427722056917d5e3a19d64ca36bc65aa6b5b85b 
>   ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
>   ui/src/main/sass/components/_diff.scss 
> f58f7bde33e123a50ade0026a35d880593c4274b 
> 
> 
> Diff: https://reviews.apache.org/r/63092/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Finer grained diff
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63088: Add pointer for pagination links

[David McLaughlin]

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

Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
Shanmugham.


Repository: aurora


Description
---

Add pointer for pagination links


Diffs
-

  ui/src/main/sass/components/_pagination.scss 
337249145948e73b13f6d9b2a44936ca4490348b 


Diff: https://reviews.apache.org/r/63088/diff/1/


Testing
---

Tested in vagrant.


Thanks,

David McLaughlin

Re: Review Request 63088: Add pointer for pagination links

[Santhosh Kumar Shanmugham]

> On Oct. 17, 2017, 1:42 p.m., Santhosh Kumar Shanmugham wrote:
> > Ship It!

Link an image?


- Santhosh Kumar


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


On Oct. 17, 2017, 1:39 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63088/
> ---
> 
> (Updated Oct. 17, 2017, 1:39 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add pointer for pagination links
> 
> 
> Diffs
> -
> 
>   ui/src/main/sass/components/_pagination.scss 
> 337249145948e73b13f6d9b2a44936ca4490348b 
> 
> 
> Diff: https://reviews.apache.org/r/63088/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63088: Add pointer for pagination links

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 1:39 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63088/
> ---
> 
> (Updated Oct. 17, 2017, 1:39 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add pointer for pagination links
> 
> 
> Diffs
> -
> 
>   ui/src/main/sass/components/_pagination.scss 
> 337249145948e73b13f6d9b2a44936ca4490348b 
> 
> 
> Diff: https://reviews.apache.org/r/63088/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63088: Add pointer for pagination links

[David McLaughlin]

> On Oct. 17, 2017, 8:42 p.m., Santhosh Kumar Shanmugham wrote:
> > Ship It!
> 
> Santhosh Kumar Shanmugham wrote:
> Link an image?

I would but you can't easily take a picture of your cursor on a Mac.


- David


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


On Oct. 17, 2017, 8:39 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63088/
> ---
> 
> (Updated Oct. 17, 2017, 8:39 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add pointer for pagination links
> 
> 
> Diffs
> -
> 
>   ui/src/main/sass/components/_pagination.scss 
> 337249145948e73b13f6d9b2a44936ca4490348b 
> 
> 
> Diff: https://reviews.apache.org/r/63088/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 62956: Immediately reject offers lacking necessary resources

[Stephan Erb]

> On Oct. 17, 2017, 8:26 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> > Lines 67-68 (patched)
> > 
> >
> > Since the Scheduler fails over every 24 hours, maybe we can let the new 
> > Scheduler retry the Slave?
> > 
> > 30 days seems like a very high threshold and can sneak into a tight 
> > capacity situation without much warning. Typically in those scenarios, we 
> > manually churn the cluster to free up space. Wonder how the 30 day filter 
> > would behave in such a case. Having said that, we should make this 
> > configurable with a resonable default (few hrs)?
> 
> Jordan Ly wrote:
> I believe that the filter only works against a specific framework ID, so 
> that a scheduler failover or deploy would receive the offers again.
> 
> Jordan Ly wrote:
> Additionally, does churning the cluster mean new offers would be 
> generated? If so, I think that they would get a new offer ID and be reissued.

The framework ID remains constent across failovers of both Aurora schedulers 
and Mesos masters. Otherwise we'd lose all currently runnings tasks during a 
failover.

For the filtering I am under the impression that it is per agent and 
independent of offer or offer IDs. To be safe, we should check with some Mesos 
developers though :)


- Stephan


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


On Oct. 13, 2017, 1:18 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62956/
> ---
> 
> (Updated Oct. 13, 2017, 1:18 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There's no reason for us to evaluate offers with no CPUs or memory, so reject 
> them early in the offer lifecycle.
> 
> This is an incremental performance optimization, but it may net significant 
> improvements based on observations in some very large clusters.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 3c77e2983ce00f897f3d5ed106b779cd7f7f0940 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> e8334310a2a46a0ccb09ee6e4122c515892d3996 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  1b1239753f40d7d46d91724def6c25037eb79f1c 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java 
> d5db81b88a0369d0b26c8fbf70efab3886ad7695 
>   src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java 
> b98aaaf48ae60afef19a368ee96abc897300f8fa 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/offers/Offers.java 
> 129b4437315c6ad4ea47ca75d4ae6e28cadd7911 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 765a527acb96997989c920be8b69dfa1113dc302 
> 
> 
> Diff: https://reviews.apache.org/r/62956/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 1:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 1:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63087: Fix instance range display

[Santhosh Kumar Shanmugham]

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


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 17, 2017, 1:25 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63087/
> ---
> 
> (Updated Oct. 17, 2017, 1:25 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix instance range display
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/utils/Task.js 3259623091957ea39500e6b1caa79ac4b8a3d78d 
>   ui/src/main/js/utils/__tests__/Task-test.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63087/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:test
> ./gradlew ui:lint
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-17 at 1.24.03 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/88a998ef-6cd1-4620-bfd8-85ebf792984a__Screen_Shot_2017-10-17_at_1.24.03_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[David McLaughlin]

> On Oct. 17, 2017, 9:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
> Currently that would have to assume the executor config is JSON.

But my long term plan is to look for "AuroraExecutor" in the config and do nice 
things for Thermos (and allow other people to drop in their own parsers for 
custom executors).


- David


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


On Oct. 17, 2017, 8:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[Stephan Erb]

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



Should we consider pretty printing the executor configuration (when assembling 
the thrift struct in the client)? Hopefully this would yield diffs that are a 
bit easier to read.

- Stephan Erb


On Oct. 17, 2017, 10:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 10:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63083: Add diff viewer to Update Page

[David McLaughlin]

> On Oct. 17, 2017, 9:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when 
> > assembling the thrift struct in the client)? Hopefully this would yield 
> > diffs that are a bit easier to read.

Currently that would have to assume the executor config is JSON.


- David


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


On Oct. 17, 2017, 8:11 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> ---
> 
> (Updated Oct. 17, 2017, 8:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar 
> Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page 
> is that the "after" diff is fixed - it's always the desiredState. But I've 
> extracted the Diff rendering into a shared component between ConfigDiff and 
> UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ConfigDiff.js 
> 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 
> 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js 
> fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 
> 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 
> 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 
> 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss 
> a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Selecting from multiple initial configs
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63092: Detect and parse Thermos config in Diff output

[David McLaughlin]

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

(Updated Oct. 18, 2017, 12:08 a.m.)


Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
Erb.


Changes
---

Minor typo fix from another review.


Repository: aurora


Description
---

Add support for Thermos configs in Diff viewer. This deserializes the opaque 
"data" string so that the jsonDiff algorithm creates finer grained diffs.


Diffs (updated)
-

  ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
  ui/src/main/js/components/__tests__/Diff-test.js 
7f913223a3aa8ae6ce5322936e2653727481d549 
  ui/src/main/js/test-utils/TaskBuilders.js 
8427722056917d5e3a19d64ca36bc65aa6b5b85b 
  ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
  ui/src/main/sass/components/_diff.scss 
f58f7bde33e123a50ade0026a35d880593c4274b 
  ui/src/main/sass/components/_job-page.scss 
bafff882b92404f21231040c8935483f0cebfd1e 


Diff: https://reviews.apache.org/r/63092/diff/2/

Changes: https://reviews.apache.org/r/63092/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Finer grained diff
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png


Thanks,

David McLaughlin

Re: Review Request 63092: Detect and parse Thermos config in Diff output

[David McLaughlin]

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

(Updated Oct. 18, 2017, 12:16 a.m.)


Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
Erb.


Changes
---

Tweak red/green colors.


Repository: aurora


Description
---

Add support for Thermos configs in Diff viewer. This deserializes the opaque 
"data" string so that the jsonDiff algorithm creates finer grained diffs.


Diffs (updated)
-

  ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
  ui/src/main/js/components/__tests__/Diff-test.js 
7f913223a3aa8ae6ce5322936e2653727481d549 
  ui/src/main/js/test-utils/TaskBuilders.js 
8427722056917d5e3a19d64ca36bc65aa6b5b85b 
  ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
  ui/src/main/sass/components/_diff.scss 
f58f7bde33e123a50ade0026a35d880593c4274b 
  ui/src/main/sass/components/_job-page.scss 
bafff882b92404f21231040c8935483f0cebfd1e 


Diff: https://reviews.apache.org/r/63092/diff/3/

Changes: https://reviews.apache.org/r/63092/diff/2-3/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Finer grained diff
  
https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png


Thanks,

David McLaughlin

Re: Review Request 63092: Detect and parse Thermos config in Diff output

[Aurora ReviewBot]

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


Ship it!




Master (0efe415) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 12:16 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63092/
> ---
> 
> (Updated Oct. 18, 2017, 12:16 a.m.)
> 
> 
> Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
> Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for Thermos configs in Diff viewer. This deserializes the opaque 
> "data" string so that the jsonDiff algorithm creates finer grained diffs.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/Diff.js 53ba5176304bcc8d53c3964bcc7d57a20ff17e48 
>   ui/src/main/js/components/__tests__/Diff-test.js 
> 7f913223a3aa8ae6ce5322936e2653727481d549 
>   ui/src/main/js/test-utils/TaskBuilders.js 
> 8427722056917d5e3a19d64ca36bc65aa6b5b85b 
>   ui/src/main/js/utils/Task.js 7da6d1034a81afaa527ff983282a2a826b637534 
>   ui/src/main/sass/components/_diff.scss 
> f58f7bde33e123a50ade0026a35d880593c4274b 
>   ui/src/main/sass/components/_job-page.scss 
> bafff882b92404f21231040c8935483f0cebfd1e 
> 
> 
> Diff: https://reviews.apache.org/r/63092/diff/3/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Finer grained diff
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/17/f9e09012-a8b1-4f18-8d70-4947f8208de0__Screen_Shot_2017-10-17_at_3.23.12_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63098: Clean up Job Page CSS

[David McLaughlin]

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

Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Repository: aurora


Description
---

Clean up Job Page CSS. 

* Make update list smaller (was too dominant on the page).
* Show update progress/size of history.
* Tidy up whitespace. 
* Move expander to end of task list item.
* Wrap the main job overview loading element in a panel group to prevent 
jarring page change as content loads.


Diffs
-

  ui/src/main/js/components/TaskList.js 
5a61de8fc46cc729906eb756c54c208320d34da7 
  ui/src/main/js/components/UpdateList.js 
2df28394713d31da7d60b6b32028c16613bec3a0 
  ui/src/main/js/pages/Job.js 5f92ad0b801c047f6c056623069438a5aa781c02 
  ui/src/main/js/pages/__tests__/Job-test.js 
2b126b65bb09c40a528f87148942615dd035e36d 
  ui/src/main/sass/components/_task-list.scss 
a6e2f0a1994134381c6d16b05209771ab2b09988 
  ui/src/main/sass/components/_update-list.scss 
83a1f5a07291ea3aaabb8145877f5d7f8d8f433e 


Diff: https://reviews.apache.org/r/63098/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Tighter task list item. 
  
https://reviews.apache.org/media/uploaded/files/2017/10/18/8effbff4-5b1a-4883-857e-b6107ff25c6c__Screen_Shot_2017-10-17_at_8.06.09_PM.png
Job Update List
  
https://reviews.apache.org/media/uploaded/files/2017/10/18/9b23-1d3b-4eb4-94c3-cb0b332dfd95__Screen_Shot_2017-10-17_at_8.06.16_PM.png


Thanks,

David McLaughlin

Re: Review Request 63098: Clean up Job Page CSS

[Aurora ReviewBot]

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


Ship it!




Master (0efe415) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 3:29 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63098/
> ---
> 
> (Updated Oct. 18, 2017, 3:29 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Clean up Job Page CSS. 
> 
> * Make update list smaller (was too dominant on the page).
> * Show update progress/size of history.
> * Tidy up whitespace. 
> * Move expander to end of task list item.
> * Wrap the main job overview loading element in a panel group to prevent 
> jarring page change as content loads.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/TaskConfigSummary.js 
> 43b50d9e485d1d8c03572453c4c01a01c528ffe7 
>   ui/src/main/js/components/TaskList.js 
> 5a61de8fc46cc729906eb756c54c208320d34da7 
>   ui/src/main/js/components/UpdateList.js 
> 2df28394713d31da7d60b6b32028c16613bec3a0 
>   ui/src/main/js/pages/Job.js 5f92ad0b801c047f6c056623069438a5aa781c02 
>   ui/src/main/js/pages/__tests__/Job-test.js 
> 2b126b65bb09c40a528f87148942615dd035e36d 
>   ui/src/main/sass/components/_instance-page.scss 
> 99204fdfca4441d824c3dfff083f78e1d094b4c9 
>   ui/src/main/sass/components/_task-list.scss 
> a6e2f0a1994134381c6d16b05209771ab2b09988 
>   ui/src/main/sass/components/_update-list.scss 
> 83a1f5a07291ea3aaabb8145877f5d7f8d8f433e 
> 
> 
> Diff: https://reviews.apache.org/r/63098/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Tighter task list item. 
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/18/8effbff4-5b1a-4883-857e-b6107ff25c6c__Screen_Shot_2017-10-17_at_8.06.09_PM.png
> Job Update List
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/18/9b23-1d3b-4eb4-94c3-cb0b332dfd95__Screen_Shot_2017-10-17_at_8.06.16_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 63099: Add Source Sans Pro font to project

[Aurora ReviewBot]

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



This patch does not apply cleanly against master (0efe415), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 18, 2017, 4:19 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63099/
> ---
> 
> (Updated Oct. 18, 2017, 4:19 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Locally host Google Font 'Source Sans Pro' rather than have internal 
> infrastructure make external calls to Google's CDN. Will be included in the 
> Webpack bundle. 
> 
> https://fonts.google.com/specimen/Source+Sans+Pro
> 
> 
> Diffs
> -
> 
>   .gitignore 829ab055c50e02a2536054e74e27d21fbed2e2f7 
>   ui/src/main/js/index.js 5ad5f3709742777ff0c0418e8f4e1ce122ee9d2d 
>   ui/src/main/resources/SourceSansPro-Black.ttf PRE-CREATION 
>   ui/src/main/resources/SourceSansPro-Bold.ttf PRE-CREATION 
>   ui/src/main/resources/SourceSansPro-Regular.ttf PRE-CREATION 
>   ui/src/main/resources/SourceSansPro-Semibold.ttf PRE-CREATION 
>   ui/src/main/resources/source-sans-pro.css PRE-CREATION 
>   ui/webpack.config.js 789817f0d39a3bcea73492bb5eeff645a8a2cb30 
> 
> 
> Diff: https://reviews.apache.org/r/63099/diff/1/
> 
> 
> Testing
> ---
> 
> Disabled the locally installed font and ran in Vagrant. Works fine.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Review Request 63099: Add Source Sans Pro font to project

[David McLaughlin]

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

Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Repository: aurora


Description
---

Locally host Google Font 'Source Sans Pro' rather than have internal 
infrastructure make external calls to Google's CDN. Will be included in the 
Webpack bundle. 

https://fonts.google.com/specimen/Source+Sans+Pro


Diffs
-

  .gitignore 829ab055c50e02a2536054e74e27d21fbed2e2f7 
  ui/src/main/js/index.js 5ad5f3709742777ff0c0418e8f4e1ce122ee9d2d 
  ui/src/main/resources/SourceSansPro-Black.ttf PRE-CREATION 
  ui/src/main/resources/SourceSansPro-Bold.ttf PRE-CREATION 
  ui/src/main/resources/SourceSansPro-Regular.ttf PRE-CREATION 
  ui/src/main/resources/SourceSansPro-Semibold.ttf PRE-CREATION 
  ui/src/main/resources/source-sans-pro.css PRE-CREATION 
  ui/webpack.config.js 789817f0d39a3bcea73492bb5eeff645a8a2cb30 


Diff: https://reviews.apache.org/r/63099/diff/1/


Testing
---

Disabled the locally installed font and ran in Vagrant. Works fine.


Thanks,

David McLaughlin

Re: Review Request 63098: Clean up Job Page CSS

[David McLaughlin]

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

(Updated Oct. 18, 2017, 3:29 a.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Changes
---

Some instance page fixes too.


Repository: aurora


Description
---

Clean up Job Page CSS. 

* Make update list smaller (was too dominant on the page).
* Show update progress/size of history.
* Tidy up whitespace. 
* Move expander to end of task list item.
* Wrap the main job overview loading element in a panel group to prevent 
jarring page change as content loads.


Diffs (updated)
-

  ui/src/main/js/components/TaskConfigSummary.js 
43b50d9e485d1d8c03572453c4c01a01c528ffe7 
  ui/src/main/js/components/TaskList.js 
5a61de8fc46cc729906eb756c54c208320d34da7 
  ui/src/main/js/components/UpdateList.js 
2df28394713d31da7d60b6b32028c16613bec3a0 
  ui/src/main/js/pages/Job.js 5f92ad0b801c047f6c056623069438a5aa781c02 
  ui/src/main/js/pages/__tests__/Job-test.js 
2b126b65bb09c40a528f87148942615dd035e36d 
  ui/src/main/sass/components/_instance-page.scss 
99204fdfca4441d824c3dfff083f78e1d094b4c9 
  ui/src/main/sass/components/_task-list.scss 
a6e2f0a1994134381c6d16b05209771ab2b09988 
  ui/src/main/sass/components/_update-list.scss 
83a1f5a07291ea3aaabb8145877f5d7f8d8f433e 


Diff: https://reviews.apache.org/r/63098/diff/2/

Changes: https://reviews.apache.org/r/63098/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test

See screenshots.


File Attachments


Tighter task list item. 
  
https://reviews.apache.org/media/uploaded/files/2017/10/18/8effbff4-5b1a-4883-857e-b6107ff25c6c__Screen_Shot_2017-10-17_at_8.06.09_PM.png
Job Update List
  
https://reviews.apache.org/media/uploaded/files/2017/10/18/9b23-1d3b-4eb4-94c3-cb0b332dfd95__Screen_Shot_2017-10-17_at_8.06.16_PM.png


Thanks,

David McLaughlin