Re: Review Request 23872: Fix problem with deschedule command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/#review48574 --- Does this point out lack of unit test coverage, or were unit tests failing? - Bill Farner On July 23, 2014, 11:33 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/ --- (Updated July 23, 2014, 11:33 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: aurora-597 https://issues.apache.org/jira/browse/aurora-597 Repository: aurora Description --- Fix problem with deschedule command. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4e5cce0796d679aa898dde7bee3cee804540c4a9 Diff: https://reviews.apache.org/r/23872/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 23872: Fix problem with deschedule command.
It's a bit sneaky - unit tests pass, because they mock the call to the API, and the way that the API is implemented in Python, the usual spec mechanism doesn't work for the thrift proxy. So the unit test couldn't catch this. The end-to-end test should have caught it - I'm not sure why it didn't. I'll be looking into it. -Mark On Wed, Jul 23, 2014 at 7:37 PM, Bill Farner wfar...@apache.org wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/ Does this point out lack of unit test coverage, or were unit tests failing? - Bill Farner On July 23rd, 2014, 11:33 p.m. UTC, Mark Chu-Carroll wrote: Review request for Aurora and Maxim Khutornenko. By Mark Chu-Carroll. *Updated July 23, 2014, 11:33 p.m.* *Bugs: * aurora-597 https://issues.apache.org/jira/browse/aurora-597 *Repository: * aurora Description Fix problem with deschedule command. Diffs - src/main/python/apache/aurora/client/api/__init__.py (4e5cce0796d679aa898dde7bee3cee804540c4a9) View Diff https://reviews.apache.org/r/23872/diff/
Re: Review Request 23872: Fix problem with deschedule command.
wired up to thrift. The generated thrift object (AuroraAdmin.Client) has these signatures, though, right? -=Bill On Wed, Jul 23, 2014 at 4:54 PM, Mark Chu-Carroll mchucarr...@apache.org wrote: Unfortunately, no. Because the python code that calls through thrift is using a dynamic invocation method - it captures calls via __getattr__, and then dynamically invokes them on a parameter list. There's no python object or class to spec - just a class with __getattr__ wired up to thrift. -Mark On Wed, Jul 23, 2014 at 7:48 PM, Bill Farner wfar...@apache.org wrote: Would mock.mocksignature help? Seems like the biggest issue is that our mocks lose track of the original function signature. http://www.voidspace.org.uk/python/mock/mocksignature.html -=Bill On Wed, Jul 23, 2014 at 4:42 PM, Mark Chu-Carroll mchucarr...@apache.org wrote: (Slight modification of one statement from the previous: end-to-end tests *did* catch it. The question is, why did they ever pass? ) On Wed, Jul 23, 2014 at 7:40 PM, Mark Chu-Carroll mchucarr...@apache.org wrote: It's a bit sneaky - unit tests pass, because they mock the call to the API, and the way that the API is implemented in Python, the usual spec mechanism doesn't work for the thrift proxy. So the unit test couldn't catch this. The end-to-end test should have caught it - I'm not sure why it didn't. I'll be looking into it. -Mark On Wed, Jul 23, 2014 at 7:37 PM, Bill Farner wfar...@apache.org wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/ Does this point out lack of unit test coverage, or were unit tests failing? - Bill Farner On July 23rd, 2014, 11:33 p.m. UTC, Mark Chu-Carroll wrote: Review request for Aurora and Maxim Khutornenko. By Mark Chu-Carroll. *Updated July 23, 2014, 11:33 p.m.* *Bugs: * aurora-597 https://issues.apache.org/jira/browse/aurora-597 *Repository: * aurora Description Fix problem with deschedule command. Diffs - src/main/python/apache/aurora/client/api/__init__.py (4e5cce0796d679aa898dde7bee3cee804540c4a9) View Diff https://reviews.apache.org/r/23872/diff/
Re: Review Request 23872: Fix problem with deschedule command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/#review48611 --- Ship it! Ship It! - Maxim Khutornenko On July 23, 2014, 11:54 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23872/ --- (Updated July 23, 2014, 11:54 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: aurora-597 https://issues.apache.org/jira/browse/aurora-597 Repository: aurora Description --- Fix problem with deschedule command. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4e5cce0796d679aa898dde7bee3cee804540c4a9 src/test/python/apache/aurora/client/cli/test_cron.py d5f57df66eab8a7113e6e9670b4bab2352d4dbf5 Diff: https://reviews.apache.org/r/23872/diff/ Testing --- Thanks, Mark Chu-Carroll
