> On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, line 81 > > <https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line81> > > > > >=
It's safe as is but since Mark asked about this too changing it for better readability. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, line 44 > > <https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line44> > > > > i'm fine with tuning the min_poll_interval and max_poll_interval but > > possibly make them configurable via __init__, ie. __init__(..., > > min_poll_interval=MIN_POLL_INTERVAL, max_poll_interval=MAX_POLL_INTERVAL) Sure, works for me. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, line 56 > > <https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line56> > > > > as a library this should probably be log.error Done. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, lines 74-75 > > <https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line74> > > > > update the docstring to reflect new return behavior Done. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/job_monitor.py, lines 76-77 > > <https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line76> > > > > i don't like the idea of wait_until having side effects -- this > > effectively renders it not thread safe. > > > > can you clone the query and update instanceIds on the cloned query, > > then change iter_query to take a query parameter? Great catch. Fixed. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 284 > > <https://reviews.apache.org/r/20950/diff/2/?file=572974#file572974line284> > > > > pass the kwarg so that this is more self-documenting, i.e. > > with_timeout=True Done. > On May 1, 2014, 8:59 p.m., Brian Wickman wrote: > > src/main/python/apache/aurora/client/commands/core.py, lines 101-102 > > <https://reviews.apache.org/r/20950/diff/2/?file=572977#file572977line101> > > > > weird indent. > > > > also this should use die('Tasks were not killed in time.') Done and done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41947 ----------------------------------------------------------- On May 1, 2014, 8:42 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20950/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 8:42 p.m.) > > > Review request for Aurora, Mark Chu-Carroll and Brian Wickman. > > > Bugs: AURORA-370 > https://issues.apache.org/jira/browse/AURORA-370 > > > Repository: aurora > > > Description > ------- > > Migration plan: > 1. Prepare client to poll for the status of tasks being killed. This is > mostly a noop at this point while the scheduler wait with a shorter timeout > is still in place. > 2. Cut off scheduler killTasks wait thus relying on the client poll (coming > next). > > This RB addresses client changes: > - Repurposing JobMonitor to poll for all job tasks regardless of their > arrival time. > - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching > the max_poll_interval. > - Setting the JobMonitor max_poll_interval to be higher than the scheduler > killTasks backoff to ensure smooth migration. > - Added wait_util calls into kill/killall/updater calls. > > Once this ticket is closed, the next step would be to make kill/killall truly > async: AURORA-371 > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/BUILD > 32097d220a6f8a58555260ce081ace0c58b9fa99 > src/main/python/apache/aurora/client/api/job_monitor.py > b694ef62ba708c4b67c5cc931b0edfd93702027f > src/main/python/apache/aurora/client/api/updater.py > 0acf45034db124ed369d0bcb8095fe5545605eed > src/main/python/apache/aurora/client/cli/jobs.py > 782b34800def55c54b67ca5f13da66c4997a0777 > src/main/python/apache/aurora/client/cli/task.py > 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 > src/main/python/apache/aurora/client/commands/core.py > 39190e06f3eac1ab4dabf48055418bc383be14ed > src/test/python/apache/aurora/client/api/BUILD > dd9b79764db0bbc3a11c51051e43b93a9b5d370a > src/test/python/apache/aurora/client/api/test_job_monitor.py > 32609e4e97afc7b8b164f3db438c60efd0ebbe11 > src/test/python/apache/aurora/client/api/test_updater.py > e7eb1e783baaabe9437e55ecb848f6a42de13dd7 > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 > src/test/python/apache/aurora/client/cli/test_create.py > 875573e2ee534ea50da71fac2174ecc8877a714d > src/test/python/apache/aurora/client/cli/test_kill.py > cf5df648206e0cc234d603c1fd56dc7ab1311fa9 > src/test/python/apache/aurora/client/cli/test_plugins.py > 2dab749d1dcba99a1e3c792c8f26ee86441673d1 > src/test/python/apache/aurora/client/cli/test_update.py > cf077e821ebe32104b3e1345014f5c7b07c44c34 > src/test/python/apache/aurora/client/cli/util.py > 2225ab071dff72be547c43f66ffdc890171c3288 > src/test/python/apache/aurora/client/commands/test_create.py > e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 > src/test/python/apache/aurora/client/commands/test_kill.py > db820a543b0ea11e5a40cc2a00209aa8fa6186c9 > src/test/python/apache/aurora/client/commands/test_update.py > 6e145db9ff8213c8f098b53b9a9668532d664249 > > Diff: https://reviews.apache.org/r/20950/diff/ > > > Testing > ------- > > ./pants src/test/python:all > > > Thanks, > > Maxim Khutornenko > >