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 2014/02/20 02:15:10 UTC
Review Request 18296: Adding admin_client sla_list_safe_domain command.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/
-----------------------------------------------------------
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Bugs: AURORA-208
https://issues.apache.org/jira/browse/AURORA-208
Repository: aurora
Description
-------
Implemented admin_client sla_list_safe_domain command.
Diffs
-----
src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
Diff: https://reviews.apache.org/r/18296/diff/
Testing
-------
./pants src/test/python/apache/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 21, 2014, 2:55 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/api/sla.py, line 88
> > <https://reviews.apache.org/r/18296/diff/1/?file=498483#file498483line88>
> >
> > This comment doesn't make sense to me. I don't understand what an instance of this class is. Is it a list of hosts?
> >
> > Maybe start the comment with what the thing is, and then explain what it does?
Fair suggestion. Commented on the class purpose and get_safe_hosts algorithm.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35150
-----------------------------------------------------------
On Feb. 20, 2014, 1:15 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 1:15 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35150
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65563>
This comment doesn't make sense to me. I don't understand what an instance of this class is. Is it a list of hosts?
Maybe start the comment with what the thing is, and then explain what it does?
- Mark Chu-Carroll
On Feb. 19, 2014, 8:15 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 8:15 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35168
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65595>
for clarity i'd name this 'for job_key in jobs' since you're iterating over the keys, not the jobs.
alternately you could do
for job_key, job in jobs.items():
...
so that you don't need to keep doing self._jobs[job_key]
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65593>
you can use a for/else statement here:
for job in jobs:
<compute statistics>
if statistics below threshold:
break
else:
safe_hosts[host].append(safe_limits)
the 'else' statement will only be executed if no breaks took place, so you don't need to manage the 'safe' boolean.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65596>
shouldn't this be safe_hosts[host] = safe_limits?
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65585>
2 newlines between toplevel defns. here and below in this file
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65586>
don't do this in a comprehension. just
for line in results:
print(line)
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65587>
i'd add a safety clause here to skip empty lines since i know my editor has the propensity to add these to the ends of files:
if not line.strip():
continue
src/test/python/apache/aurora/client/commands/test_admin_sla.py
<https://reviews.apache.org/r/18296/#comment65584>
no need to do nested patches here:
with patch(...) as mock_options:
...
- Brian Wickman
On Feb. 21, 2014, 4:30 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 4:30 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35170
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65589>
Same comment as class below.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65588>
Minor stylistic nit: for a class, you should say what it is, for a method, what it does. So this would read better as "A grouping of all active tasks by:"...
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65590>
Would help if this comment said a bit more - I don't know what the override map values mean. Can they arbitrarily change the limits in either direction? Or only make them stricter?
- Mark Chu-Carroll
On Feb. 21, 2014, 11:30 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 11:30 a.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/
-----------------------------------------------------------
(Updated Feb. 21, 2014, 11:10 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
CR comments.
Bugs: AURORA-208
https://issues.apache.org/jira/browse/AURORA-208
Repository: aurora
Description
-------
Implemented admin_client sla_list_safe_domain command.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
Diff: https://reviews.apache.org/r/18296/diff/
Testing
-------
./pants src/test/python/apache/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 21, 2014, 11 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/sla.py, line 142
> > <https://reviews.apache.org/r/18296/diff/3/?file=500295#file500295line142>
> >
> > it looks like this was never updated. safe_hosts[host] = safe_limits != self_hosts[host].append(safe_limits)
> >
> > do you mean .extend()?
Sorry, missed it with the "else" reversal. Fixed now.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35203
-----------------------------------------------------------
On Feb. 21, 2014, 7:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 7:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35203
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65654>
it looks like this was never updated. safe_hosts[host] = safe_limits != self_hosts[host].append(safe_limits)
do you mean .extend()?
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65655>
it looks like this was never updated. safe_hosts[host] = safe_limits != self_hosts[host].append(safe_limits)
do you mean .extend()?
- Brian Wickman
On Feb. 21, 2014, 7:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 7:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35202
-----------------------------------------------------------
Ship it!
Ship It!
- Brian Wickman
On Feb. 21, 2014, 7:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 7:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35186
-----------------------------------------------------------
Ship it!
Ship It!
- Mark Chu-Carroll
On Feb. 21, 2014, 2:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 2:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/
-----------------------------------------------------------
(Updated Feb. 21, 2014, 7:49 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
CR comments.
Bugs: AURORA-208
https://issues.apache.org/jira/browse/AURORA-208
Repository: aurora
Description
-------
Implemented admin_client sla_list_safe_domain command.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
Diff: https://reviews.apache.org/r/18296/diff/
Testing
-------
./pants src/test/python/apache/aurora/client:all
Thanks,
Maxim Khutornenko
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Brian Wickman <wi...@apache.org>.
> On Feb. 21, 2014, 7:48 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/sla.py, lines 135-139
> > <https://reviews.apache.org/r/18296/diff/2/?file=500057#file500057line135>
> >
> > The else would fire even if the _hosts is empty producing an unbound variable error. I would have to add an extra check before the loop for this to work, which I'd rather not.
i don't think so, right? the 'if safe' is in the inner loop where safe_limits is defined as []
- Brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35182
-----------------------------------------------------------
On Feb. 21, 2014, 7:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 7:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Feb. 21, 2014, 7:48 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/sla.py, lines 135-139
> > <https://reviews.apache.org/r/18296/diff/2/?file=500057#file500057line135>
> >
> > The else would fire even if the _hosts is empty producing an unbound variable error. I would have to add an extra check before the loop for this to work, which I'd rather not.
>
> Brian Wickman wrote:
> i don't think so, right? the 'if safe' is in the inner loop where safe_limits is defined as []
You're right. I somehow misinterpreted it for the outer loop. Fixed.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35182
-----------------------------------------------------------
On Feb. 21, 2014, 7:49 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 7:49 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/#review35182
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65609>
Done.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65608>
Done.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65610>
Done.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65611>
Done.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65615>
Renamed to job_key.
The for loop refactoring would not work as self._job[job_key] gets a set of tasks that are not available in the _hosts map.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65619>
The else would fire even if the _hosts is empty producing an unbound variable error. I would have to add an extra check before the loop for this to work, which I'd rather not.
src/main/python/apache/aurora/client/api/sla.py
<https://reviews.apache.org/r/18296/#comment65621>
Sure, that'll work too.
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65624>
Done.
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65625>
Done.
src/main/python/apache/aurora/client/commands/admin.py
<https://reviews.apache.org/r/18296/#comment65626>
Sure, done.
src/test/python/apache/aurora/client/commands/test_admin_sla.py
<https://reviews.apache.org/r/18296/#comment65627>
Dropped.
- Maxim Khutornenko
On Feb. 21, 2014, 4:30 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18296/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 4:30 p.m.)
>
>
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
>
>
> Bugs: AURORA-208
> https://issues.apache.org/jira/browse/AURORA-208
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implemented admin_client sla_list_safe_domain command.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
> src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
> src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
> src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
> src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
> src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
> src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
>
> Diff: https://reviews.apache.org/r/18296/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python/apache/aurora/client:all
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 18296: Adding admin_client sla_list_safe_domain command.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18296/
-----------------------------------------------------------
(Updated Feb. 21, 2014, 4:30 p.m.)
Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
Changes
-------
CR comments.
Bugs: AURORA-208
https://issues.apache.org/jira/browse/AURORA-208
Repository: aurora
Description
-------
Implemented admin_client sla_list_safe_domain command.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/__init__.py 0999b6f99bd74361663258cdae9d3920b3bb636f
src/main/python/apache/aurora/client/api/sla.py b0b2b245951ea72d113ff2b3969c4d54024f73e7
src/main/python/apache/aurora/client/commands/admin.py 45686aec8d69f0dfa1d649a92a19ca87bd315823
src/test/python/apache/aurora/client/api/test_sla.py 2e6c8faf668182b497df59fe41b74af240459de7
src/test/python/apache/aurora/client/commands/BUILD 02c27aa513d60ef901f446c56cbfc0d891445e30
src/test/python/apache/aurora/client/commands/test_admin_sla.py PRE-CREATION
src/test/python/apache/aurora/client/commands/util.py 11ae990c0d8e41470c6df438d37d8a2a79907a58
Diff: https://reviews.apache.org/r/18296/diff/
Testing
-------
./pants src/test/python/apache/aurora/client:all
Thanks,
Maxim Khutornenko