You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/08/24 19:33:01 UTC

Review Request 37719: Revocable: schema changes.

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

Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description
-------

Added support for 'tier' in schema and thrift.


Diffs
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
  src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
  src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
  src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
  src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
  src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
  src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
  src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
  src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
  src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 

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


Testing
-------

./gradlew -Pq build
./pants test.pytest --no-fast src/test/python::
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.

> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 255
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048455#file1048455line255>
> >
> >     Will this attribute be required in the future?
> 
> Maxim Khutornenko wrote:
>     Absolutely. Otherwise, how will users specify their tier? We have debated using env as a replacement but there are just too many drawbacks with it. See "alernative solution" at the end of design doc: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo
> 
> Bill Farner wrote:
>     I'm a -1 to requiring it via the API.  There are plenty of valid use cases where there are not multiple tier definitions, and i would like to support a minimal input over the API for those scenarios.
>     
>     However, we can continue this discussion when the time is right.
> 
> Maxim Khutornenko wrote:
>     Not sure I understand your proposal. How will users instruct Aurora to use a specific tier without this change?

I'm advocating for `tier` to always be optional via the scheduler API.


- Bill


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


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.

> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 255
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048455#file1048455line255>
> >
> >     Will this attribute be required in the future?
> 
> Maxim Khutornenko wrote:
>     Absolutely. Otherwise, how will users specify their tier? We have debated using env as a replacement but there are just too many drawbacks with it. See "alernative solution" at the end of design doc: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo

I'm a -1 to requiring it via the API.  There are plenty of valid use cases where there are not multiple tier definitions, and i would like to support a minimal input over the API for those scenarios.

However, we can continue this discussion when the time is right.


- Bill


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


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 255
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048455#file1048455line255>
> >
> >     Will this attribute be required in the future?

Absolutely. Otherwise, how will users specify their tier? We have debated using env as a replacement but there are just too many drawbacks with it. See "alernative solution" at the end of design doc: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo


> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/TierManager.java, line 37
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048457#file1048457line37>
> >
> >     It might be better to create a ticket for this task and put the ticket id here.

Sure, created AURORA-1443.


> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > src/main/python/apache/aurora/client/config.py, line 60
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048461#file1048461line60>
> >
> >     Please create a ticket for this task. You could also repurpose https://issues.apache.org/jira/browse/AURORA-319

This folds into AURORA-1443.


- Maxim


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 255
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048455#file1048455line255>
> >
> >     Will this attribute be required in the future?
> 
> Maxim Khutornenko wrote:
>     Absolutely. Otherwise, how will users specify their tier? We have debated using env as a replacement but there are just too many drawbacks with it. See "alernative solution" at the end of design doc: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo
> 
> Bill Farner wrote:
>     I'm a -1 to requiring it via the API.  There are plenty of valid use cases where there are not multiple tier definitions, and i would like to support a minimal input over the API for those scenarios.
>     
>     However, we can continue this discussion when the time is right.

Not sure I understand your proposal. How will users instruct Aurora to use a specific tier without this change?


- Maxim


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


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8:57 p.m., Zameer Manji wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 255
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048455#file1048455line255>
> >
> >     Will this attribute be required in the future?
> 
> Maxim Khutornenko wrote:
>     Absolutely. Otherwise, how will users specify their tier? We have debated using env as a replacement but there are just too many drawbacks with it. See "alernative solution" at the end of design doc: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo
> 
> Bill Farner wrote:
>     I'm a -1 to requiring it via the API.  There are plenty of valid use cases where there are not multiple tier definitions, and i would like to support a minimal input over the API for those scenarios.
>     
>     However, we can continue this discussion when the time is right.
> 
> Maxim Khutornenko wrote:
>     Not sure I understand your proposal. How will users instruct Aurora to use a specific tier without this change?
> 
> Bill Farner wrote:
>     I'm advocating for `tier` to always be optional via the scheduler API.

Ah, I finally got a sense of what you meant. Yes, it's not going to be *required* on every API call at either thrift or logical level. We will have a default tier to fall back to once we are fully on this model. At the same time, I expect client to default to *environment* iff the tier is not explicitly defined by the user. This will make it virtually impossible to have a null tier sent to scheduler when using Aurora client.


- Maxim


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


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/#review96199
-----------------------------------------------------------

Ship it!



api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 255)
<https://reviews.apache.org/r/37719/#comment151523>

    Will this attribute be required in the future?



src/main/java/org/apache/aurora/scheduler/TierManager.java (line 37)
<https://reviews.apache.org/r/37719/#comment151519>

    It might be better to create a ticket for this task and put the ticket id here.



src/main/python/apache/aurora/client/config.py (line 60)
<https://reviews.apache.org/r/37719/#comment151522>

    Please create a ticket for this task. You could also repurpose https://issues.apache.org/jira/browse/AURORA-319


- Zameer Manji


On Aug. 24, 2015, 10:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 10:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 25, 2015, 12:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/TierManager.java, line 39
> > <https://reviews.apache.org/r/37719/diff/2/?file=1048730#file1048730line39>
> >
> >     I suggest the 'prod' constant go in api.thrift.

This will require deprecation cycle and prevent Aurora operators from customizing their tier names. The idea here to not hardcode tier names anywhere but the tiers.yaml (or tiers.json?) file. While 'prod' might make sense as a default environment value, I'd defer adding any thrift constants until we tackle environment handling in Aurora (open issue in the design doc).


> On Aug. 25, 2015, 12:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java, line 66
> > <https://reviews.apache.org/r/37719/diff/2/?file=1048731#file1048731line66>
> >
> >     Maybe `"tier-" + job.getEnvironment()` to ensure env/tier wires aren't crossed.

Done.


- Maxim


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


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/#review96229
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/TierManager.java (line 38)
<https://reviews.apache.org/r/37719/#comment151552>

    I suggest the 'prod' constant go in api.thrift.



src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java (line 66)
<https://reviews.apache.org/r/37719/#comment151553>

    Maybe `"tier-" + job.getEnvironment()` to ensure env/tier wires aren't crossed.


- Bill Farner


On Aug. 24, 2015, 9:21 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/#review96438
-----------------------------------------------------------

Ship it!


Ship It!

- Bill Farner


On Aug. 25, 2015, 3:28 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2015, 3:28 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/
-----------------------------------------------------------

(Updated Aug. 25, 2015, 10:28 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Removing hardcoded tier mapping. Will rather spend time fully implementing TierManager next to properly translate tier definitions.


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


Repository: aurora


Description
-------

Added support for 'tier' in schema and thrift.


Diffs (updated)
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
  src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
  src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
  src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
  src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
  src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
  src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
  src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
  src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 

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


Testing
-------

./gradlew -Pq build
./pants test.pytest --no-fast src/test/python::
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko


Re: Review Request 37719: Revocable: schema changes.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/#review96278
-----------------------------------------------------------

Ship it!


Master (f202f2d) 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 Aug. 25, 2015, 12:54 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2015, 12:54 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/
-----------------------------------------------------------

(Updated Aug. 25, 2015, 12:54 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


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


Repository: aurora


Description
-------

Added support for 'tier' in schema and thrift.


Diffs (updated)
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
  src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
  src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
  src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
  src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
  src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
  src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
  src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
  src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 

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


Testing
-------

./gradlew -Pq build
./pants test.pytest --no-fast src/test/python::
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/
-----------------------------------------------------------

(Updated Aug. 24, 2015, 9:21 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Address Bill's and Zameer's comments.


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


Repository: aurora


Description
-------

Added support for 'tier' in schema and thrift.


Diffs (updated)
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
  src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
  src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
  src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
  src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
  src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
  src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
  src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
  src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
  src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 

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


Testing
-------

./gradlew -Pq build
./pants test.pytest --no-fast src/test/python::
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.
> 
> Bill Farner wrote:
>     What's not clear is why our tutorial has to change for this.

We use `environment = 'prod'` but don't set the production flag thus defaulting to non-prod behavior. This change fixes the oversight and delivers on naming expectations. Nothing else in the tutorial has to change.


> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 191
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048465#file1048465line191>
> >
> >     consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.
> 
> Maxim Khutornenko wrote:
>     I thought about that but ruled it out in the end. If applied, the default will make it impossible to run non-prod tasks with regular (non-revocable) resources. The default 'noop' resource estimator in Mesos (1) does not send any revocable offers. Thus, we need a tri-state here to try out oversubscription and gradually move towards revocable-only usage for non-prod tasks.
>     
>     Once we are confident oversubscription works as intended (later in 0.10.0 or even 0.11.0) we'll make the switch to use tier-default everywhere (both client schema and scheduler). Makes sense?
>     
>     (1) - https://github.com/apache/mesos/blob/f68661e75b8fd09311e0fcd057e34ae836e30f94/docs/oversubscription.md
> 
> Bill Farner wrote:
>     Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?  I recognize the need for >=3 states, but i figured that would be handled with different/varying tier definiitions.

| Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?

Correct. This is the long(er) term view. It does not have to be tied to this feature though. We need to design and carefully orchestrate that client/env -> server/tier migration making sure we deprecate things gracefully. I expect to address this shortly after this feature is complete.


- Maxim


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Zameer Manji <zm...@apache.org>.

> On Aug. 24, 2015, 1 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 191
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048465#file1048465line191>
> >
> >     consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.
> 
> Maxim Khutornenko wrote:
>     I thought about that but ruled it out in the end. If applied, the default will make it impossible to run non-prod tasks with regular (non-revocable) resources. The default 'noop' resource estimator in Mesos (1) does not send any revocable offers. Thus, we need a tri-state here to try out oversubscription and gradually move towards revocable-only usage for non-prod tasks.
>     
>     Once we are confident oversubscription works as intended (later in 0.10.0 or even 0.11.0) we'll make the switch to use tier-default everywhere (both client schema and scheduler). Makes sense?
>     
>     (1) - https://github.com/apache/mesos/blob/f68661e75b8fd09311e0fcd057e34ae836e30f94/docs/oversubscription.md
> 
> Bill Farner wrote:
>     Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?  I recognize the need for >=3 states, but i figured that would be handled with different/varying tier definiitions.
> 
> Maxim Khutornenko wrote:
>     | Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?
>     
>     Correct. This is the long(er) term view. It does not have to be tied to this feature though. We need to design and carefully orchestrate that client/env -> server/tier migration making sure we deprecate things gracefully. I expect to address this shortly after this feature is complete.

I think adding a comment explaining the lack of NON NULL here would be very helpful.


- Zameer


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


On Aug. 24, 2015, 10:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 10:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Zameer Manji <zm...@apache.org>.

> On Aug. 24, 2015, 1 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.
> 
> Bill Farner wrote:
>     What's not clear is why our tutorial has to change for this.
> 
> Maxim Khutornenko wrote:
>     We use `environment = 'prod'` but don't set the production flag thus defaulting to non-prod behavior. This change fixes the oversight and delivers on naming expectations. Nothing else in the tutorial has to change.

I don't mind this change, but it does feel out of place in the diff.


- Zameer


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


On Aug. 24, 2015, 10:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 10:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.
> 
> Bill Farner wrote:
>     What's not clear is why our tutorial has to change for this.
> 
> Maxim Khutornenko wrote:
>     We use `environment = 'prod'` but don't set the production flag thus defaulting to non-prod behavior. This change fixes the oversight and delivers on naming expectations. Nothing else in the tutorial has to change.
> 
> Zameer Manji wrote:
>     I don't mind this change, but it does feel out of place in the diff.
> 
> Bill Farner wrote:
>     Doesn't this mean, though, that quota needs to be applied to the user?
> 
> Zameer Manji wrote:
>     Good point. Maxim, would you mind making this change in a different review?

Yeah, quota is a valid concern. Reverted.


> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 191
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048465#file1048465line191>
> >
> >     consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.
> 
> Maxim Khutornenko wrote:
>     I thought about that but ruled it out in the end. If applied, the default will make it impossible to run non-prod tasks with regular (non-revocable) resources. The default 'noop' resource estimator in Mesos (1) does not send any revocable offers. Thus, we need a tri-state here to try out oversubscription and gradually move towards revocable-only usage for non-prod tasks.
>     
>     Once we are confident oversubscription works as intended (later in 0.10.0 or even 0.11.0) we'll make the switch to use tier-default everywhere (both client schema and scheduler). Makes sense?
>     
>     (1) - https://github.com/apache/mesos/blob/f68661e75b8fd09311e0fcd057e34ae836e30f94/docs/oversubscription.md
> 
> Bill Farner wrote:
>     Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?  I recognize the need for >=3 states, but i figured that would be handled with different/varying tier definiitions.
> 
> Maxim Khutornenko wrote:
>     | Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?
>     
>     Correct. This is the long(er) term view. It does not have to be tied to this feature though. We need to design and carefully orchestrate that client/env -> server/tier migration making sure we deprecate things gracefully. I expect to address this shortly after this feature is complete.
> 
> Zameer Manji wrote:
>     I think adding a comment explaining the lack of NON NULL here would be very helpful.

Added a TODO linking to AURORA-1443.


- Maxim


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.

> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.

What's not clear is why our tutorial has to change for this.


> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 191
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048465#file1048465line191>
> >
> >     consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.
> 
> Maxim Khutornenko wrote:
>     I thought about that but ruled it out in the end. If applied, the default will make it impossible to run non-prod tasks with regular (non-revocable) resources. The default 'noop' resource estimator in Mesos (1) does not send any revocable offers. Thus, we need a tri-state here to try out oversubscription and gradually move towards revocable-only usage for non-prod tasks.
>     
>     Once we are confident oversubscription works as intended (later in 0.10.0 or even 0.11.0) we'll make the switch to use tier-default everywhere (both client schema and scheduler). Makes sense?
>     
>     (1) - https://github.com/apache/mesos/blob/f68661e75b8fd09311e0fcd057e34ae836e30f94/docs/oversubscription.md

Perhaps i'm missing something...isn't the idea that server-side tier definitions allow this type of configuration?  I recognize the need for >=3 states, but i figured that would be handled with different/varying tier definiitions.


- Bill


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?

There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.


> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 191
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048465#file1048465line191>
> >
> >     consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.

I thought about that but ruled it out in the end. If applied, the default will make it impossible to run non-prod tasks with regular (non-revocable) resources. The default 'noop' resource estimator in Mesos (1) does not send any revocable offers. Thus, we need a tri-state here to try out oversubscription and gradually move towards revocable-only usage for non-prod tasks.

Once we are confident oversubscription works as intended (later in 0.10.0 or even 0.11.0) we'll make the switch to use tier-default everywhere (both client schema and scheduler). Makes sense?

(1) - https://github.com/apache/mesos/blob/f68661e75b8fd09311e0fcd057e34ae836e30f94/docs/oversubscription.md


- Maxim


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.

> On Aug. 24, 2015, 8 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.
> 
> Bill Farner wrote:
>     What's not clear is why our tutorial has to change for this.
> 
> Maxim Khutornenko wrote:
>     We use `environment = 'prod'` but don't set the production flag thus defaulting to non-prod behavior. This change fixes the oversight and delivers on naming expectations. Nothing else in the tutorial has to change.
> 
> Zameer Manji wrote:
>     I don't mind this change, but it does feel out of place in the diff.

Doesn't this mean, though, that quota needs to be applied to the user?


- Bill


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


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Zameer Manji <zm...@apache.org>.

> On Aug. 24, 2015, 1 p.m., Bill Farner wrote:
> > examples/jobs/hello_world.aurora, line 29
> > <https://reviews.apache.org/r/37719/diff/1/?file=1048456#file1048456line29>
> >
> >     Revert?
> 
> Maxim Khutornenko wrote:
>     There was a `production = True` kvarg added that required to reformat here. This is to enforce production behavior in view of the current and upcoming tier-related work.
> 
> Bill Farner wrote:
>     What's not clear is why our tutorial has to change for this.
> 
> Maxim Khutornenko wrote:
>     We use `environment = 'prod'` but don't set the production flag thus defaulting to non-prod behavior. This change fixes the oversight and delivers on naming expectations. Nothing else in the tutorial has to change.
> 
> Zameer Manji wrote:
>     I don't mind this change, but it does feel out of place in the diff.
> 
> Bill Farner wrote:
>     Doesn't this mean, though, that quota needs to be applied to the user?

Good point. Maxim, would you mind making this change in a different review?


- Zameer


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


On Aug. 24, 2015, 10:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 10:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 37719: Revocable: schema changes.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37719/#review96185
-----------------------------------------------------------



examples/jobs/hello_world.aurora (line 29)
<https://reviews.apache.org/r/37719/#comment151506>

    Revert?



src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql (line 191)
<https://reviews.apache.org/r/37719/#comment151507>

    consider `NOT NULL` here, and apply the implicit tier=defaut behavior in the scheduler.


- Bill Farner


On Aug. 24, 2015, 5:32 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37719/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:32 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1414
>     https://issues.apache.org/jira/browse/AURORA-1414
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added support for 'tier' in schema and thrift.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/jobs/hello_world.aurora 5401bfebe753b5e53abd08baeac501144ced9b5a 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java ebfad9788a65fbfb7790e40db4a47a6a570b4a7b 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 1903315c0753c68fd1e103d48fff037ba59b7642 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java d103d19b30c5e219c385018d26d6872464520380 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 956c508b163cadb13858f7ac0fbc7a971c2cc770 
>   src/main/python/apache/aurora/client/config.py 59703ef18c61dbed635954e05a38385ac364b679 
>   src/main/python/apache/aurora/config/schema/base.py 214d5594a2c22e92d5412e40c2ddf18e65c2af63 
>   src/main/python/apache/aurora/config/thrift.py adf53bb1c28d61e9bcb670c60f293cf8262c5836 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml bfad339a84c6ca87bc6fea339af10a559d8eb3d5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 7634047abaec9129ee9ead08cc51a10b3261515d 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 37e19ac71b0f1da7b8a0c06137e43d2143302d85 
>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b231827ff7fdb2aa37580a32cae74d5da17c2f97 
>   src/test/python/apache/aurora/client/cli/test_create.py 69039b6c0504c9d14a96693249c7199958aadc96 
>   src/test/python/apache/aurora/client/test_config.py 986061bf0829caa0509416a3de1778c2fa40a766 
>   src/test/python/apache/aurora/config/test_thrift.py 061864eb475807332d328fa72f35f179d36ef9e8 
> 
> Diff: https://reviews.apache.org/r/37719/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./pants test.pytest --no-fast src/test/python::
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>