> On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 237 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line237> > > > > 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)
You can't signal from a non-main thread but apparently any thread can catch KeyboardInterrupt. This wasted a few hours of my life until I ran into this "little" caveat (https://docs.python.org/2/library/thread.html?highlight=keyboardinterrupt): "Threads interact strangely with interrupts: the KeyboardInterrupt exception will be received by an arbitrary thread. (When the signal module is available, interrupts always go to the main thread.)" > On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 224 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line224> > > > > snake_case please Done. > On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 64 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line64> > > > > since you're creating unbounded queues, this is equivalent to .put() > > (here and elsewhere) Changed. > On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 23 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line23> > > > > minor nit, this should be > > > > try: > > from Queue import Queue, Empty > > except ImportError: > > from queue import Queue, Empty > > > > (yay 2.x/3.x) Done. > On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/instance_watcher.py, line 97 > > <https://reviews.apache.org/r/21440/diff/1/?file=581726#file581726line97> > > > > while not self._terminating: Done. > On May 14, 2014, 10:57 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, lines 93-94 > > <https://reviews.apache.org/r/21440/diff/1/?file=581727#file581727line93> > > > > 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. Thanks, I see now what you meant. Great suggestion. Done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/#review43046 ----------------------------------------------------------- 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 > >
