Fixing bug with jsonrpc._Method caused by debugger

[efokschaner]

Whilst debugging jsonrpclib with PyCharm I observed odd behaviour with jsonrpc._Method due to pycharm using the repr, str and dir methods in order to populate its debug information. These calls would be handled by getattr which would modify the object, corrupting the method name chain. xmlrpclib does not have this issue because it does not make the same optimisation of mutating the existing object but returns a new object when getattr is called (this might actually be a preferable approach to what i am proposing, so consider it as another option).

UPDATE:
After working with the library a bit more i think it would be preferable to revert back to the xmlrpclib approach of not mutating _Method and returning a a new instance each time getattr is called. Because it allows code such as this to work:

foo_proxy = jsonrpclib.Server('http://localhost:80')
remote_bar = foo_proxy.objects.bar
remote_bar.baz()
remote_bar.qux()

If we mutate the _Method then the 'qux()' call would not behave as expected.

Let me know if there are any questions or concerns.

[efokschaner]

@joshmarshall I noticed you're active in the last few days so wanted to necro this and get your thoughts on this issue.

Is it not worth fixing?

[joshmarshall]

Hi @efokschaner -

I think it is worth fixing. It looks like this is still the first implementation -- did you want to open a PR with the immutable _Method() approach?

[efokschaner]

@joshmarshall Thanks for the reply, I'll prepare the PR with the recommended fix.

[efokschaner]

@joshmarshall I've updated the PR with the __gettattr__ change. I decided to keep the __repr__, __str__ and __dir__ implementations as it still provides a better experience.

[efokschaner]

@joshmarshall How does this look?

[joshmarshall]

@efokschaner
The change looks good, thanks for making it -- however, I think it needs a test to verify the change you made (that you referenced originally -- the varying behavior based on method lookup). On that note, I've opened a pull request (here) that ties into Travis for automated testing. If you (and anyone else) wants to review that, I can merge into master. You can go ahead and merge that branch in here and update the tests, and it will run in Travis too.

Thoughts?

[efokschaner]

@joshmarshall
Thanks, I'll take a look at your PR and then update this one with tests.

[joshmarshall]

@efokschaner Alright -- 0.1.4 is released on PyPI and in Travis. Let's make this 0.1.5. :)

[efokschaner]

@joshmarshall Test is added, let me know how it looks.

[joshmarshall]

@efokschaner Hey -- the test looks good. Sorry, I had a pip install issue with the setup.py + the automated Travis push to PyPI last week, so I had to deal with that. Will merge this fix in as 0.1.7 hopefully this weekend if that works for you.

[efokschaner]

@joshmarshall fine by me!

[efokschaner]

@joshmarshall You might prefer to merge #43 first.

[efokschaner]

@joshmarshall Don't forget this one :)

[efokschaner]

@joshmarshall any eta on this merge?

[efokschaner]

@joshmarshall Just gonna bump this PR, let me know if you're too busy to deal with the merge.

[joshmarshall]

Only took me two years, but it's closed now. Thanks so much for your work and patience!

[efokschaner]

@joshmarshall Outstanding, you should see my face right now :D