> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/instance_watcher.py, line 48
> > <https://reviews.apache.org/r/21440/diff/7-9/?file=604778#file604778line48>
> >
> >     this should be terminating_event=None
> >     
> >     and self._terminating = terminating_event or Event()
> >     
> >     otherwise you have a shared singleton Event object across all 
> > InstaceWatchers which is probably not what you want

I actually always need it as it's now being used to wait on instead of 
_clock.sleep() (line 128). 


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/job_monitor.py, line 39
> > <https://reviews.apache.org/r/21440/diff/7-9/?file=604779#file604779line39>
> >
> >     same comment re:Event

Same here. It's now a required part of the job_monitor.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/scheduler_mux.py, line 122
> > <https://reviews.apache.org/r/21440/diff/9/?file=616435#file616435line122>
> >
> >     it's more idiomatic/consistent with the rest of the codebase to make 
> > Error be inside the class, i.e.
> >     
> >     class SchedulerMux(object):
> >       class Error(Exception): pass
> >     
> >     then raising self.Error

Sure, done.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/task_util.py, lines 41-46
> > <https://reviews.apache.org/r/21440/diff/9/?file=616436#file616436line41>
> >
> >     inline the returns, e.g.
> >     
> >     if self._scheduler_mux:
> >        return ...
> >     else:
> >        return ...

Done.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/task_util.py, line 52
> > <https://reviews.apache.org/r/21440/diff/9/?file=616436#file616436line52>
> >
> >     not optional unless you do something like instance_ids=None in the defn
> >     
> >     then furthermore you can be a little more specific in the include_ids 
> > method
> >     
> >     include_ids = lambda id: id in instance_ids if instance_ids is not None 
> > else True

Fixed.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/client/api/task_util.py, lines 90-91
> > <https://reviews.apache.org/r/21440/diff/9/?file=616436#file616436line90>
> >
> >     this is copypasta from apache/aurora/client/base.py -- maybe add a 
> > format_response method there so that you can just do 
> > log.debug(format_response(resp))
> >     
> >

Makes sense. Done.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/test/python/apache/aurora/client/api/test_scheduler_mux.py, line 68
> > <https://reviews.apache.org/r/21440/diff/9/?file=616442#file616442line68>
> >
> >     Amount(1, Time.MILLISECONDS) to make this faster

This is testing queue.get() timeout, which is set in seconds. Tried and it did 
not make anything faster.


> On June 25, 2014, 10:44 p.m., Brian Wickman wrote:
> > src/test/python/apache/aurora/client/api/test_task_util.py, lines 48-50
> > <https://reviews.apache.org/r/21440/diff/9/?file=616443#file616443line48>
> >
> >     why the def create_task?  the following is <100:
> >     
> >     return [ScheduledTask(assignedTask=AssignedTask(instanceId=index)) for 
> > index in cls.INSTANCES]

It used to be longer. I swear! 


- Maxim


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


On June 24, 2014, 11:28 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21440/
> -----------------------------------------------------------
> 
> (Updated June 24, 2014, 11:28 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/BUILD 
> c205a7d5f281c2b9e52d778c3d84647f25dafc15 
>   src/main/python/apache/aurora/client/api/error_handling_thread.py 
> PRE-CREATION 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> e09aa9a6c32c17f13c9b8ff3a589919587bd839b 
>   src/main/python/apache/aurora/client/api/job_monitor.py 
> d176995fca68d42fcc2d3989483eaf520d0d737f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 10a956c50de893c8380ef1f763ed7d6686b5c00c 
>   src/main/python/apache/aurora/client/api/scheduler_mux.py PRE-CREATION 
>   src/main/python/apache/aurora/client/api/task_util.py PRE-CREATION 
>   src/main/python/apache/aurora/client/api/updater.py 
> c592651770f9426fd42b7f92fe2185e1e3065c43 
>   src/main/python/apache/aurora/client/api/updater_util.py 
> 6b689c11b86c99ef5538c0b7bf4dbb81e0df2d13 
>   src/test/python/apache/aurora/client/api/BUILD 
> 804195b719cad7652b3ccb15f03c7c80fc2a13f3 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> 723a5b6f429f85202ee4ad3a004f019cec632a98 
>   src/test/python/apache/aurora/client/api/test_job_monitor.py 
> 3aa96075e085ac7c2607a8c303ee5daf6e3e43b0 
>   src/test/python/apache/aurora/client/api/test_scheduler_mux.py PRE-CREATION 
>   src/test/python/apache/aurora/client/api/test_task_util.py PRE-CREATION 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 48f82c8d9606b7e3bae77eaabbbca8edc92ab045 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> b9313be657ebaf9c79a7695b75abe2d90a7d75dd 
>   src/test/python/apache/aurora/client/cli/test_kill.py 
> e9e167991cd06e556dbbc839eb9a808eb7d88fe9 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 407eb41eb6e78721ef29baefdee5699e605193f9 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 44a180e88893fea1d7acf1862d5aaa95d98598f5 
>   src/test/python/apache/aurora/client/cli/util.py 
> a2c9d09c58243250baefab20000ee51f5ea3b411 
>   src/test/python/apache/aurora/client/commands/test_create.py 
> 4cd1343b19e9d13abe20d3b88dc7460ec512d57b 
>   src/test/python/apache/aurora/client/commands/test_kill.py 
> 94bbe1bfff1c8d333837aa6bb14700714f46288b 
>   src/test/python/apache/aurora/client/commands/test_restart.py 
> 0c6d5a069c43d96098c126aa340a734cb37c667a 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> ea8e092267fd65ccd24ca1be3c04f50ba89f5236 
>   src/test/python/apache/aurora/client/commands/util.py 
> b1822f27d2890469efefc5bfa0878f29163c167a 
>   src/test/python/apache/aurora/client/fake_scheduler_proxy.py 
> 2a4773c81efb390385f675854e9631500b263a45 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> 80871273fc4d47558253e6b09c92724e8693bc11 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> fc723cf232ddbc10458fc394e37358c8523118c2 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 9c5652829ac306dda5f7e95e164c85713e18988f 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 
> 2af256d65850bd861111279dff4b5c53f234cf7a 
> 
> 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