Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Jonathan Boulle]

> On Feb. 3, 2014, 6:50 p.m., Jonathan Boulle wrote:
> > src/main/python/apache/aurora/client/commands/admin.py, lines 174-181
> > 
> >
> > hmm, log.error is nonfatal, so you should probably die() in these two 
> > cases

sorry I didn't catch this earlier :-(


- Jonathan


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


On Jan. 31, 2014, 7:52 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 7:52 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Jonathan Boulle]

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



src/main/python/apache/aurora/client/commands/admin.py


hmm, log.error is nonfatal, so you should probably die() in these two cases


- Jonathan Boulle


On Jan. 31, 2014, 7:52 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 7:52 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Bill Farner]

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

Ship it!


Looks like Mark merged this to master, it's 4abccc3.  Thanks again!

- Bill Farner


On Jan. 31, 2014, 7:52 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 7:52 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Mark Chu-Carroll]

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

Ship it!


Ship It!

- Mark Chu-Carroll


On Jan. 31, 2014, 2:52 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 2:52 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Dan Norris]

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

(Updated Jan. 31, 2014, 7:52 p.m.)


Review request for Aurora and Mark Chu-Carroll.


Changes
---

Changed indentation from 4 spaces to 2 spaces (didn't have my vimrc set up 
correctly on that machine).


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


Repository: aurora


Description
---

AURORA-108: make set_quota in aurora_admin require explicit units

set_quota now requies a specific unit (M or G) as input.

This means that arguments to set_quote should resemble the following:
set_quota(, , , '50M', '20G')
set_quota(, , , '50G', '20G')


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
f82900371f8e7566da07beeef4ff0622105c64d6 
  src/main/python/apache/aurora/client/commands/admin.py 
6d191f406a284c60326a49aed32914c81522d4d2 

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


Testing
---

Passes local tests. I could not find a test that tests the quota for the cli 
client so this patch does not contain an dedicated test case.


Thanks,

Dan Norris

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Jonathan Boulle]

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



src/main/python/apache/aurora/client/commands/admin.py


whoah - these indents appear to be 4sp, should be 2sp


- Jonathan Boulle


On Jan. 31, 2014, 7:34 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 7:34 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Mark Chu-Carroll]

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

Ship it!


Ship It!

- Mark Chu-Carroll


On Jan. 31, 2014, 2:34 p.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 2:34 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Dan Norris]

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

(Updated Jan. 31, 2014, 7:34 p.m.)


Review request for Aurora and Mark Chu-Carroll.


Changes
---

Updated patch to use parse_data from twitter.common.


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


Repository: aurora


Description
---

AURORA-108: make set_quota in aurora_admin require explicit units

set_quota now requies a specific unit (M or G) as input.

This means that arguments to set_quote should resemble the following:
set_quota(, , , '50M', '20G')
set_quota(, , , '50G', '20G')


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
f82900371f8e7566da07beeef4ff0622105c64d6 
  src/main/python/apache/aurora/client/commands/admin.py 
6d191f406a284c60326a49aed32914c81522d4d2 

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


Testing
---

Passes local tests. I could not find a test that tests the quota for the cli 
client so this patch does not contain an dedicated test case.


Thanks,

Dan Norris

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Dan Norris]

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

(Updated Jan. 31, 2014, 6:46 p.m.)


Review request for Aurora and Mark Chu-Carroll.


Changes
---

Added markcc


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


Repository: aurora


Description
---

AURORA-108: make set_quota in aurora_admin require explicit units

set_quota now requies a specific unit (M or G) as input.

This means that arguments to set_quote should resemble the following:
set_quota(, , , '50M', '20G')
set_quota(, , , '50G', '20G')


Diffs
-

  src/main/python/apache/aurora/client/api/__init__.py 
f82900371f8e7566da07beeef4ff0622105c64d6 
  src/main/python/apache/aurora/client/commands/admin.py 
6d191f406a284c60326a49aed32914c81522d4d2 

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


Testing
---

Passes local tests. I could not find a test that tests the quota for the cli 
client so this patch does not contain an dedicated test case.


Thanks,

Dan Norris

Re: Review Request 17581: AURORA-108: make set_quota in aurora_admin require explicit units

[Bill Farner]

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


Thanks a ton for the patch!  Aurora leans heavily on open-sourced common 
libraries from Twitter, and there's actually a convenience to assist with this. 
 Unfortunately the only way i know to poke around at these is in the 
interpreter:

$ ./pants py 3rdparty/python:twitter.common.quantity
>>> from twitter.common.quantity.parse_simple import parse_data
>>> help(parse_data)
Help on function parse_data in module twitter.common.quantity.parse_simple:

parse_data(datastring)
Parse a data string of the format:
  [integer][unit]
where unit is in upper/lowercase k, kb, m, mb, g, gb, t, tb

>>> from twitter.common.quantity import Data
>>> parse_data('100tb')
Amount(100, TB)
>>> parse_data('100tb').as_(Data.GB)
102400.0

You can invoke the tool manually to gain confidence in the code:
$ cat ~/.aurora/clusters.json
[{
  "name": "example",
  "auth_mechanism": "none",
  "scheduler_uri": "localhost:5"
}]
$ ./pants py ./src/main/python/apache/aurora/client/bin:aurora_admin set_quota 
example fake_role 1 2 3
 INFO] Setting quota for user:fake_role cpu:1.00 ram_mb:2 disk_mb: 3

You'll likely see a hang here since you're not running a scheduler, but you can 
see the values.

As for unit test coverage, there might be some non-trivial hoops to jump 
through (more than i'd want to subject you to).  However, could you add markcc 
to the People line to see if he has any advice?


src/main/python/apache/aurora/client/commands/admin.py


This might illustrate the syntax better:

  usage: set_quota cluster role cpu ram[MGT] disk[MGT]

at least, that matches man page convention.


- Bill Farner


On Jan. 31, 2014, 6:26 a.m., Dan Norris wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17581/
> ---
> 
> (Updated Jan. 31, 2014, 6:26 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-136
> https://issues.apache.org/jira/browse/AURORA-136
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-108: make set_quota in aurora_admin require explicit units
> 
> set_quota now requies a specific unit (M or G) as input.
> 
> This means that arguments to set_quote should resemble the following:
> set_quota(, , , '50M', '20G')
> set_quota(, , , '50G', '20G')
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> f82900371f8e7566da07beeef4ff0622105c64d6 
>   src/main/python/apache/aurora/client/commands/admin.py 
> 6d191f406a284c60326a49aed32914c81522d4d2 
> 
> Diff: https://reviews.apache.org/r/17581/diff/
> 
> 
> Testing
> ---
> 
> Passes local tests. I could not find a test that tests the quota for the cli 
> client so this patch does not contain an dedicated test case.
> 
> 
> Thanks,
> 
> Dan Norris
> 
>