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


a few high level comments.  still need to wade through the semantics since it's 
a big change.


src/main/python/apache/aurora/client/api/instance_watcher.py
<https://reviews.apache.org/r/21440/#comment77043>

    while not self._terminating:



src/main/python/apache/aurora/client/api/job_monitor.py
<https://reviews.apache.org/r/21440/#comment77044>

    the reason that I like self._terminating to be a threading.Event is because 
then you could do:
    
    self._terminating.wait(poll_interval.as_(Time.SECONDS))
    
    which means that you will break out of the loop fast if self.terminate() is 
called from another thread.



src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/21440/#comment77054>

    minor nit, this should be
    
    try:
      from Queue import Queue, Empty
    except ImportError:
      from queue import Queue, Empty
    
    (yay 2.x/3.x)



src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/21440/#comment77046>

    since you're creating unbounded queues, this is equivalent to .put() (here 
and elsewhere)



src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/21440/#comment77048>

    snake_case please



src/main/python/apache/aurora/client/api/updater.py
<https://reviews.apache.org/r/21440/#comment77050>

    it surprises me that this is necessary.  will signal.signal() even work 
outside of the MainThread?  i'm pretty sure OSes will raise an exception if you 
try (e.g. Linux)


- Brian Wickman


On May 14, 2014, 6:49 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21440/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 6:49 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
> 
> 
> Bugs: AURORA-350
>     https://issues.apache.org/jira/browse/AURORA-350
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The updater now spawns upto batch_size threads to process one instance per 
> thread. 
> 
> All mutating calls are multiplexed by the SchedulerMux to do batch 
> kill/add/restart calls. This is the first step towards a fully multiplexed 
> SchedulerProxy and is intended to mitigate LDAP/scheduler load.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> 8992b93d25ce71b5784bad24429fd32356f64763 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> a27f6fbbf38e981f259aea35c9bf979d2d9b1b87 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ff5d2bc52af75ff2d2eee2d9b72c8d9a0df12581 
>   src/main/python/apache/aurora/client/api/updater.py 
> e8692ea4fdf924fb1ea06803d5e7321340754442 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> 5ea96cd2797c4957d05168fb7ba9f950fd82bbbf 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 03a14186646daede689ac77e2ab7144aa5a7fa14 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> a357c44c87065474bce47fa403ddd72625da072d 
>   src/test/python/apache/aurora/client/cli/util.py 
> 9d51a2e1fe961d4b522ac5b70f0737130326abe9 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 0b1e3498311d946631452e45ea2c7edb100dc43c 
>   src/test/python/apache/aurora/client/commands/util.py 
> fc9e2f17537e1e731381b825ac4b5646e5130654 
>   src/test/python/apache/aurora/client/fake_scheduler_proxy.py 
> 6a1f78b97d67adab0cf00acc31f20c5d6197a508 
>   src/test/sh/org/apache/aurora/e2e/flask/flask_example.aurora 
> ad5aafec1f796baef766ed12b2a597f26997d5b3 
>   src/test/sh/org/apache/aurora/e2e/flask/flask_example_updated.aurora 
> 6d54463c349699946c2063385b1bd38a0f8c9e58 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> c8fa07a5426c6c7748412062c26996b84637efb3 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 
> 3c4165c8464b42f7f5dc4f94a079ff7c3086cdbc 
> 
> Diff: https://reviews.apache.org/r/21440/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python:all
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to