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



Great to see how little code was necessary here! Your refactoring has really 
paid off.

Documentation seems to be the only major missing piece:

* Release notes
* Mentioning of GPU resources here: 
https://github.com/apache/aurora/blob/master/docs/features/resource-isolation.md
* Addition of GPU as a supported type: 
https://github.com/apache/aurora/blob/master/docs/reference/configuration.md#resource-object
 

Given that how cool and unique that feature is, we could also consider adding a 
more prominent user-facing documentation. We currently lack the description of 
adhoc jobs. Maybe it is worth adding a high-level documentation for them 
(similar to our services documentation 
https://github.com/apache/aurora/blob/master/docs/features/services.md) which 
uses GPU resources within the example.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 237 - 239)
<https://reviews.apache.org/r/47869/#comment199922>

    Those now deprecated in favor of `set<Resource> resources`, correct? If 
yes, please leave a deprecation note here and in the changelog.



api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 271 - 276)
<https://reviews.apache.org/r/47869/#comment199923>

    Same as above.


- Stephan Erb


On May 26, 2016, 3:20 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47869/
> -----------------------------------------------------------
> 
> (Updated May 26, 2016, 3:20 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch adds support for the Mesos GPU resource. While we have to migrate 
> to Mesos 0.29.0 for this change to take effect, nothing prevents us from 
> supporting it end-to-end in Aurora now.
> 
> I have also refactored the `configSummary.html` to display all resources 
> (including ports) in the same section. The table is populated dinamically 
> with optional resources (GPU, PORTS) hidden if no values are provided for 
> them.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> a99889c1f2d9e10825f87ea669532ad78641880f 
>   examples/vagrant/mesos_config/etc_mesos-slave/resources 
> 5bfe779fbb98822d0c58dd92e34765c5586946db 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> 6a4f110ff461876ca14c24947f4813d5f2a0dae5 
>   src/main/python/apache/aurora/config/thrift.py 
> 81a505550314c9c41f00f7c5f5bd9e093b6199c6 
>   src/main/python/apache/thermos/config/schema_base.py 
> a6768e67189b0560afef844d6b269bed8ada5f2f 
>   src/main/python/apache/thermos/config/schema_helpers.py 
> 46394bb8ffcd8b75a23d8d3ad2113f4fa1eacad2 
>   src/main/resources/scheduler/assets/configSummary.html 
> 36df616babf9a391fa3a6b5b4ff0e49ae412ea2d 
>   src/main/resources/scheduler/assets/js/filters.js 
> 98f786ef50cb8b2e0a086853c639f2d180270e15 
>   
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java 
> a5dda25a4fbbafba6baa814d28bba96f51049125 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 8769db34f2bb8cc3a52ef5c3ef95b14ee808a57e 
>   src/test/python/apache/thermos/config/test_schema.py 
> 0440ee525a58f7c1fd79babf0c8a1c8320cde80a 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> 219c40fb94561f0a390cac16e643bf4332c51aad 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 08553e4f48f137e0455ad07287086311171c06bd 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> 8b3a50ba6de992560593987f3db254baa4d29a41 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> abe0ca75c6a2c0ace15fce68ad0e5c9aa98193a4 
> 
> Diff: https://reviews.apache.org/r/47869/diff/
> 
> 
> Testing
> -------
> 
> unit and e2e tests
> 
> 
> File Attachments
> ----------------
> 
> config_summary.png
>   
> https://reviews.apache.org/media/uploaded/files/2016/05/26/ea0c14e2-f968-4223-8ce0-af942346547b__config_summary.png
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to