Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-05-20 Thread Stephan Erb 


> On April 29, 2016, 3:20 a.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.
> 
> Bill Farner wrote:
> I thought we had concluded that tier would not become a required field in 
> user configurations?
> 
> Amol Deshmukh wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> That is correct! :)
> 
> To put this change in perspective, as was previously agreed upon:
> 1. Aurora users should *never* be required to specify a tier if the 
> default tier suffices for their use. This behavior is similar to how the 
> production flag is defaulted to false.
> 2. The definition of a default tier is managed by the cluster operator 
> since the tier definition file now requires explicitly selecting a "default" 
> tier in the tier configuration file.
> 
> Given the above, the thought was that making the tier visible in the UI 
> would help socialize the tier concept as well as provide the insight that a 
> 'default' tier was in use even when the job configuration did not explicitly 
> specify a tier.
> 
> There's 2 other changes I intend to make shortly that will help clarify 
> this from the users' perspective:
> 1. AURORA-1656 "Document tier concept" (filed by serb): I intend to add 
> documentation regarding how the user could get visibility into the tier used 
> for their active jobs. I figured having the UI in place would make that 
> easier.
> 2. AURORA-1686 "Provide visibility into available tiers" (filed moments 
> ago, by me): This will allow hyperlinking the displayed tier to the tier 
> configuration page so that users get better informed about the implication of 
> their tier selection.
> 
> Let me know if this sounds like a reasonable approach to proceed with.
> 
> Maxim Khutornenko wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> Right, this is my understanding as well. By 'required' I meant the 
> non-empty thrift value, not user configuration.
> 
> I am still under opinion we should not over-advertise the Tier concept 
> until we set the default tier in the client (and/or populate it in the 
> scheduler). When we do that, I'd rather see the real assigned tier (e.g. 
> 'preemtible') instead of an anonymous 'default' one. I think having a job 
> config Tier entry should be enough of introduction into the Tier concept at 
> this point without spooking our users by a new Tier column filled with a 
> meaningless 'default' or otherwise completely empty.
> 
> Bill Farner wrote:
> > By 'required' I meant the non-empty thrift value, not user 
> configuration.
> 
> Slight tangent, but you should consider leaving the thrift value blank as 
> well.  Otherwise, it seems like it would be difficult for an operator to 
> change the default and have it apply to previously-created jobs.
> 
> Amol Deshmukh wrote:
> Ah, we were just discussing this very aspect here, internally. I think at 
> this point, the discussion/recommendation would be best captured in the gdoc 
> for this feature. For now I will:
> 1. Park this review.
> 2. Update the gdoc for tier management and send out a notification on the 
> dev list.

The discussion seems to have stalled. How do we want to proceed?


- Stephan


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


On April 29, 2016, 4:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 4:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-29 Thread Amol Deshmukh 


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.
> 
> Bill Farner wrote:
> I thought we had concluded that tier would not become a required field in 
> user configurations?
> 
> Amol Deshmukh wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> That is correct! :)
> 
> To put this change in perspective, as was previously agreed upon:
> 1. Aurora users should *never* be required to specify a tier if the 
> default tier suffices for their use. This behavior is similar to how the 
> production flag is defaulted to false.
> 2. The definition of a default tier is managed by the cluster operator 
> since the tier definition file now requires explicitly selecting a "default" 
> tier in the tier configuration file.
> 
> Given the above, the thought was that making the tier visible in the UI 
> would help socialize the tier concept as well as provide the insight that a 
> 'default' tier was in use even when the job configuration did not explicitly 
> specify a tier.
> 
> There's 2 other changes I intend to make shortly that will help clarify 
> this from the users' perspective:
> 1. AURORA-1656 "Document tier concept" (filed by serb): I intend to add 
> documentation regarding how the user could get visibility into the tier used 
> for their active jobs. I figured having the UI in place would make that 
> easier.
> 2. AURORA-1686 "Provide visibility into available tiers" (filed moments 
> ago, by me): This will allow hyperlinking the displayed tier to the tier 
> configuration page so that users get better informed about the implication of 
> their tier selection.
> 
> Let me know if this sounds like a reasonable approach to proceed with.
> 
> Maxim Khutornenko wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> Right, this is my understanding as well. By 'required' I meant the 
> non-empty thrift value, not user configuration.
> 
> I am still under opinion we should not over-advertise the Tier concept 
> until we set the default tier in the client (and/or populate it in the 
> scheduler). When we do that, I'd rather see the real assigned tier (e.g. 
> 'preemtible') instead of an anonymous 'default' one. I think having a job 
> config Tier entry should be enough of introduction into the Tier concept at 
> this point without spooking our users by a new Tier column filled with a 
> meaningless 'default' or otherwise completely empty.
> 
> Bill Farner wrote:
> > By 'required' I meant the non-empty thrift value, not user 
> configuration.
> 
> Slight tangent, but you should consider leaving the thrift value blank as 
> well.  Otherwise, it seems like it would be difficult for an operator to 
> change the default and have it apply to previously-created jobs.

Ah, we were just discussing this very aspect here, internally. I think at this 
point, the discussion/recommendation would be best captured in the gdoc for 
this feature. For now I will:
1. Park this review.
2. Update the gdoc for tier management and send out a notification on the dev 
list.


- Amol


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


On April 28, 2016, 7:54 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 7:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-29 Thread Bill Farner 


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.
> 
> Bill Farner wrote:
> I thought we had concluded that tier would not become a required field in 
> user configurations?
> 
> Amol Deshmukh wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> That is correct! :)
> 
> To put this change in perspective, as was previously agreed upon:
> 1. Aurora users should *never* be required to specify a tier if the 
> default tier suffices for their use. This behavior is similar to how the 
> production flag is defaulted to false.
> 2. The definition of a default tier is managed by the cluster operator 
> since the tier definition file now requires explicitly selecting a "default" 
> tier in the tier configuration file.
> 
> Given the above, the thought was that making the tier visible in the UI 
> would help socialize the tier concept as well as provide the insight that a 
> 'default' tier was in use even when the job configuration did not explicitly 
> specify a tier.
> 
> There's 2 other changes I intend to make shortly that will help clarify 
> this from the users' perspective:
> 1. AURORA-1656 "Document tier concept" (filed by serb): I intend to add 
> documentation regarding how the user could get visibility into the tier used 
> for their active jobs. I figured having the UI in place would make that 
> easier.
> 2. AURORA-1686 "Provide visibility into available tiers" (filed moments 
> ago, by me): This will allow hyperlinking the displayed tier to the tier 
> configuration page so that users get better informed about the implication of 
> their tier selection.
> 
> Let me know if this sounds like a reasonable approach to proceed with.
> 
> Maxim Khutornenko wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> Right, this is my understanding as well. By 'required' I meant the 
> non-empty thrift value, not user configuration.
> 
> I am still under opinion we should not over-advertise the Tier concept 
> until we set the default tier in the client (and/or populate it in the 
> scheduler). When we do that, I'd rather see the real assigned tier (e.g. 
> 'preemtible') instead of an anonymous 'default' one. I think having a job 
> config Tier entry should be enough of introduction into the Tier concept at 
> this point without spooking our users by a new Tier column filled with a 
> meaningless 'default' or otherwise completely empty.

> By 'required' I meant the non-empty thrift value, not user configuration.

Slight tangent, but you should consider leaving the thrift value blank as well. 
 Otherwise, it seems like it would be difficult for an operator to change the 
default and have it apply to previously-created jobs.


- Bill


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


On April 28, 2016, 7:54 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 7:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-29 Thread Maxim Khutornenko 


> On April 29, 2016, 1:20 a.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.
> 
> Bill Farner wrote:
> I thought we had concluded that tier would not become a required field in 
> user configurations?
> 
> Amol Deshmukh wrote:
> > I thought we had concluded that tier would not become a required field 
> in user configurations?
> 
> That is correct! :)
> 
> To put this change in perspective, as was previously agreed upon:
> 1. Aurora users should *never* be required to specify a tier if the 
> default tier suffices for their use. This behavior is similar to how the 
> production flag is defaulted to false.
> 2. The definition of a default tier is managed by the cluster operator 
> since the tier definition file now requires explicitly selecting a "default" 
> tier in the tier configuration file.
> 
> Given the above, the thought was that making the tier visible in the UI 
> would help socialize the tier concept as well as provide the insight that a 
> 'default' tier was in use even when the job configuration did not explicitly 
> specify a tier.
> 
> There's 2 other changes I intend to make shortly that will help clarify 
> this from the users' perspective:
> 1. AURORA-1656 "Document tier concept" (filed by serb): I intend to add 
> documentation regarding how the user could get visibility into the tier used 
> for their active jobs. I figured having the UI in place would make that 
> easier.
> 2. AURORA-1686 "Provide visibility into available tiers" (filed moments 
> ago, by me): This will allow hyperlinking the displayed tier to the tier 
> configuration page so that users get better informed about the implication of 
> their tier selection.
> 
> Let me know if this sounds like a reasonable approach to proceed with.

> I thought we had concluded that tier would not become a required field in 
> user configurations?

Right, this is my understanding as well. By 'required' I meant the non-empty 
thrift value, not user configuration.

I am still under opinion we should not over-advertise the Tier concept until we 
set the default tier in the client (and/or populate it in the scheduler). When 
we do that, I'd rather see the real assigned tier (e.g. 'preemtible') instead 
of an anonymous 'default' one. I think having a job config Tier entry should be 
enough of introduction into the Tier concept at this point without spooking our 
users by a new Tier column filled with a meaningless 'default' or otherwise 
completely empty.


- Maxim


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


On April 29, 2016, 2:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 2:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-29 Thread Amol Deshmukh 


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.
> 
> Bill Farner wrote:
> I thought we had concluded that tier would not become a required field in 
> user configurations?

> I thought we had concluded that tier would not become a required field in 
> user configurations?

That is correct! :)

To put this change in perspective, as was previously agreed upon:
1. Aurora users should *never* be required to specify a tier if the default 
tier suffices for their use. This behavior is similar to how the production 
flag is defaulted to false.
2. The definition of a default tier is managed by the cluster operator since 
the tier definition file now requires explicitly selecting a "default" tier in 
the tier configuration file.

Given the above, the thought was that making the tier visible in the UI would 
help socialize the tier concept as well as provide the insight that a 'default' 
tier was in use even when the job configuration did not explicitly specify a 
tier.

There's 2 other changes I intend to make shortly that will help clarify this 
from the users' perspective:
1. AURORA-1656 "Document tier concept" (filed by serb): I intend to add 
documentation regarding how the user could get visibility into the tier used 
for their active jobs. I figured having the UI in place would make that easier.
2. AURORA-1686 "Provide visibility into available tiers" (filed moments ago, by 
me): This will allow hyperlinking the displayed tier to the tier configuration 
page so that users get better informed about the implication of their tier 
selection.

Let me know if this sounds like a reasonable approach to proceed with.


- Amol


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


On April 28, 2016, 7:54 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 7:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Bill Farner 


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.
> 
> Joshua Cohen wrote:
> I'd be ok with just not shipping this change until TaskConfig.tier is 
> required and users are actually aware of it.

I thought we had concluded that tier would not become a required field in user 
configurations?


- Bill


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


On April 28, 2016, 7:54 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 7:54 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen 


> On April 29, 2016, 1:20 a.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?
> 
> Amol Deshmukh wrote:
> I thought that leaving it blank would raise more questions for most users 
> who did not use tiers at all (basically the same point that Joshua raised 
> above).
> 
> I'll leave it blank and we can address that if the need arises.

I'd be ok with just not shipping this change until TaskConfig.tier is required 
and users are actually aware of it.


- Joshua


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


On April 29, 2016, 2:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 2:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Aurora ReviewBot 

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



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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On April 29, 2016, 2:54 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 29, 2016, 2:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Amol Deshmukh 

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

(Updated April 28, 2016, 7:54 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Removed the 'default' text replacement.


Repository: aurora


Description
---

AURORA-1458: Add tier into the UI "show config" summary.


Diffs (updated)
-

  src/main/resources/scheduler/assets/configSummary.html 
1af7511de0e8a143c8ea88377aad756b44e3ac30 
  src/main/resources/scheduler/assets/js/controllers.js 
84417ebeadfae57d55b9f12e8a985825bd620fc8 
  src/main/resources/scheduler/assets/js/services.js 
d9ce52065f9573b0aa68a95da7da7c50fb14310a 
  src/main/resources/scheduler/assets/schedulingDetail.html 
eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 

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


Testing
---

Ensured the changes appear in the UI after launching the scheduler using 
vagrant.


File Attachments (updated)


Jobs by Role
  
https://reviews.apache.org/media/uploaded/files/2016/04/29/6e58d26d-e77c-49fe-85e0-ee1acae3efe0__Jobs_by_Role.png
Per Job Config Summary
  
https://reviews.apache.org/media/uploaded/files/2016/04/29/8a666fe7-9a35-427f-b1ee-f41e5413059d__Per_Job_Config_Summary.png


Thanks,

Amol Deshmukh

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Amol Deshmukh 


> On April 28, 2016, 6:20 p.m., Maxim Khutornenko wrote:
> > src/main/resources/scheduler/assets/js/controllers.js, line 139
> > 
> >
> > I am not too keen on having a column for an optional field that will 
> > most likely read 'default' for all jobs. Perhaps postpone its introduction 
> > until TaskConfig.tier is required?

I thought that leaving it blank would raise more questions for most users who 
did not use tiers at all (basically the same point that Joshua raised above).

I'll leave it blank and we can address that if the need arises.


- Amol


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


On April 28, 2016, 4:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 4:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Maxim Khutornenko 

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




src/main/resources/scheduler/assets/js/controllers.js (line 139)


I am not too keen on having a column for an optional field that will most 
likely read 'default' for all jobs. Perhaps postpone its introduction until 
TaskConfig.tier is required?


- Maxim Khutornenko


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen 

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


Ship it!




Ship It!

- Joshua Cohen


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Joshua Cohen 

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



Is there ever a case where we wouldn't want to show the tier? I.e., can someone 
configure the scheduler to *not* use tiers, or are they always on now?

I'm concerned that this is fairly prominent UI placement for a concept that 
will be completely unknown to users. It's hard for me to judge what the best UI 
is to represent tiers when no one's actually using them yet (e.g. will users 
think in terms of tier names, or will they think in terms of the properties 
associated with tiers, in which case, would it make more sense to break it down 
to the level of preemptible and revocable).

I guess for now, given the unknowns this is fine and we can iterate as we get 
feedback from users.

- Joshua Cohen


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>

Re: Review Request 46803: AURORA-1458: Add tier into the UI "show config" summary.

2016-04-28 Thread Aurora ReviewBot 

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



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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On April 28, 2016, 11:09 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46803/
> ---
> 
> (Updated April 28, 2016, 11:09 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1458: Add tier into the UI "show config" summary.
> 
> 
> Diffs
> -
> 
>   src/main/resources/scheduler/assets/configSummary.html 
> 1af7511de0e8a143c8ea88377aad756b44e3ac30 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 84417ebeadfae57d55b9f12e8a985825bd620fc8 
>   src/main/resources/scheduler/assets/js/services.js 
> d9ce52065f9573b0aa68a95da7da7c50fb14310a 
>   src/main/resources/scheduler/assets/schedulingDetail.html 
> eb88c1e6dec7a26643e8b13ffcf8e90df70a67f7 
> 
> Diff: https://reviews.apache.org/r/46803/diff/
> 
> 
> Testing
> ---
> 
> Ensured the changes appear in the UI after launching the scheduler using 
> vagrant.
> 
> 
> File Attachments
> 
> 
> Jobs by Role
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/7cbec002-e213-4c24-92d9-16b45efb839c__Jobs_by_Role.png
> Per Job Config Summary
>   
> https://reviews.apache.org/media/uploaded/files/2016/04/28/5b785fa7-1053-4b8b-ba0d-438a9170c756__Per_Job_Config_Summary.png
> 
> 
> Thanks,
> 
> Amol Deshmukh
> 
>