Yeah, but the argument list of that object is different from the argument
list used by the python client code - the dynamic stuff adds parameters for
session IDs and auth keys.

I spent a lot of time when I was writing the unit tests on this - it's a
very hard thing to mock well. There's a lot of interactions between the
underlying thrift stuff and the dynamic invocation - so we need to mock at
the SchedulerProxy level. (If we try to mock at the AuroraAdmin.Client
level, we could get a spec, but the mocking would end up needing to cover
things like @functool.wraps and __getattr__, which would create a
complicated, brittle mess.)

This really is a place where we really don't have a spec. We could create
one manually, or tweak the thrift generators - but I thought the first was
too brittle, and the second too time-consuming given the other stuff that
needed to be done. This kind of error is still able to be caught by testing
- just not by the unit testing spec-checking.

    -Mark






On Wed, Jul 23, 2014 at 7:56 PM, Bill Farner <wfar...@apache.org> wrote:

> 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/>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

Reply via email to