Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/#review65325 --- Is this needed given that the scheduler has been dual writing since

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Maxim Khutornenko
On Dec. 17, 2014, 3:34 p.m., Bill Farner wrote: Is this needed given that the scheduler has been dual writing since 0.5.0? I have explained in the ticket. We do need this change in order to avoid breaking pre-0.7.0 clients working against the 0.7.0 scheduler. - Maxim

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/ --- (Updated Dec. 17, 2014, 6:24 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29132/#review65340 --- Maybe this is actually legitimate tho? Perhaps we do want to catch

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/#review65341 --- src/main/python/apache/aurora/client/cli/jobs.py

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Brian Wickman
On Dec. 17, 2014, 6:40 p.m., Joe Smith wrote: Maybe this is actually legitimate tho? Perhaps we do want to catch PEX-INFO formatting changes? True -- this is really a bug in pex. - Brian --- This is an automatically generated

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Brian Wickman
On Dec. 17, 2014, 6:40 p.m., Joe Smith wrote: Maybe this is actually legitimate tho? Perhaps we do want to catch PEX-INFO formatting changes? Brian Wickman wrote: True -- this is really a bug in pex. that being said, it's *still* not testing the thing it should. it's testing

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Maxim Khutornenko
On Dec. 17, 2014, 6:57 p.m., Zameer Manji wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 174 https://reviews.apache.org/r/29137/diff/2/?file=794247#file794247line174 Why do we need a deep copy here? The [task] * config.instances() creates a shallow array. Without a

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Kevin Sweeney
On Dec. 17, 2014, 10:40 a.m., Joe Smith wrote: Maybe this is actually legitimate tho? Perhaps we do want to catch PEX-INFO formatting changes? Brian Wickman wrote: True -- this is really a bug in pex. Brian Wickman wrote: that being said, it's *still* not testing the thing

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Zameer Manji
On Dec. 17, 2014, 10:57 a.m., Zameer Manji wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 174 https://reviews.apache.org/r/29137/diff/2/?file=794247#file794247line174 Why do we need a deep copy here? Maxim Khutornenko wrote: The [task] * config.instances()

Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/ --- Review request for Aurora and Joe Smith. Repository: aurora Description

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/#review65350 --- src/test/python/apache/aurora/executor/test_executor_vars.py

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Maxim Khutornenko
On Dec. 17, 2014, 6:57 p.m., Zameer Manji wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 174 https://reviews.apache.org/r/29137/diff/2/?file=794247#file794247line174 Why do we need a deep copy here? Maxim Khutornenko wrote: The [task] * config.instances()

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/ --- (Updated Dec. 17, 2014, 7:26 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/#review65352 --- Ship it! Master (ab18bd0) is green with this patch.

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/#review65353 --- Ship it! Ship It! - Zameer Manji On Dec. 17, 2014, 11:26 a.m.,

Re: Review Request 29137: Implementing dual read the PopulatedJobConfig struct

2014-12-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29137/#review65355 --- Ship it! Master (ab18bd0) is green with this patch.

Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/ --- Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Brian Wickman.

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/#review65367 --- src/main/python/apache/aurora/client/factory.py

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/#review65368 --- Ship it! Master (ab18bd0) is green with this patch.

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Brian Wickman
On Dec. 17, 2014, 7:18 p.m., Kevin Sweeney wrote: src/test/python/apache/aurora/executor/test_executor_vars.py, line 34 https://reviews.apache.org/r/29171/diff/1/?file=794969#file794969line34 Can this test above be removed? should I just remove all tag exporting from the

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Kevin Sweeney
On Dec. 17, 2014, 11:18 a.m., Kevin Sweeney wrote: src/test/python/apache/aurora/executor/test_executor_vars.py, line 34 https://reviews.apache.org/r/29171/diff/1/?file=794969#file794969line34 Can this test above be removed? Brian Wickman wrote: should I just remove all tag

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/ --- (Updated Dec. 17, 2014, 8:31 p.m.) Review request for Aurora and Joe Smith.

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Kevin Sweeney
On Dec. 17, 2014, 11:18 a.m., Kevin Sweeney wrote: src/test/python/apache/aurora/executor/test_executor_vars.py, line 34 https://reviews.apache.org/r/29171/diff/1/?file=794969#file794969line34 Can this test above be removed? Brian Wickman wrote: should I just remove all tag

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/#review65377 --- Ship it! Ship It! - Kevin Sweeney On Dec. 17, 2014, 12:31 p.m.,

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Joshua Cohen
On Dec. 17, 2014, 8:07 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/factory.py, lines 27-31 https://reviews.apache.org/r/29165/diff/1/?file=795016#file795016line27 This is effecively a runtime constant, why the factory indirection? (as opposed to resolving the

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/#review65390 --- Ship it! Master (ab18bd0) is green with this patch.

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29132/#review65406 --- Ship it! Ship It! - Kevin Sweeney On Dec. 16, 2014, 4:57 p.m.,

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Maxim Khutornenko
On Dec. 17, 2014, 8:07 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/factory.py, lines 27-31 https://reviews.apache.org/r/29165/diff/1/?file=795016#file795016line27 This is effecively a runtime constant, why the factory indirection? (as opposed to resolving the

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/ --- (Updated Dec. 18, 2014, 12:59 a.m.) Review request for Aurora, Kevin Sweeney,

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/#review65423 --- Ship it! lgtm, minor comments below

Re: Review Request 29132: get_client_version should never be allowed to run unpatched in test.

2014-12-17 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29132/#review65430 --- Ship it! Ship It! - Joe Smith On Dec. 16, 2014, 4:57 p.m.,

Re: Review Request 29171: Fix test_executor_vars so that it doesn't attempt to get a real PEX-INFO.

2014-12-17 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29171/#review65431 --- Ship it! Ship It! - Joe Smith On Dec. 17, 2014, 12:31 p.m.,

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Joshua Cohen
On Dec. 18, 2014, 1:19 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, line 121 https://reviews.apache.org/r/29165/diff/2/?file=795321#file795321line121 Do you still need kwargs? Yeah, this is still used to pass user_agent to the super

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/ --- (Updated Dec. 18, 2014, 1:45 a.m.) Review request for Aurora, Kevin Sweeney,

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Maxim Khutornenko
On Dec. 18, 2014, 1:19 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/base.py, line 257 https://reviews.apache.org/r/29165/diff/2/?file=795322#file795322line257 Adding space after ';' should make it more readable. Joshua Cohen wrote: I was going for

Re: Review Request 29165: Add custom user agent for Aurora v1, Aurora v2 and Aurora Admin clients.

2014-12-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29165/#review65440 --- Ship it! Master (9042c56) is green with this patch.