Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIRequest Handler misbehaviour #134

Closed
BboyKeen opened this issue Oct 12, 2017 · 7 comments
Closed

APIRequest Handler misbehaviour #134

BboyKeen opened this issue Oct 12, 2017 · 7 comments

Comments

@BboyKeen
Copy link

BboyKeen commented Oct 12, 2017

Hi Mangopay folks,

After scratching my head a few hours digging in your code, I came to the conclusion that the way you implemented the APIRequest handler is not working.

It looks like your implementation completely ignores custom APIRequest like the one below (implementation get from the documentation) because it relies internally on the get_default_handler() method only.

import mangopay

from mangopay.api import APIRequest

handler = APIRequest(sandbox=True, timeout=60.0)

This small POC below enlights this fact by showing the references of the registered handler.

import mangopay

from mangopay.api import APIRequest
from mangopay.resources import Dispute

mangopay.client_id='your-client-id'
mangopay.passphrase='your-passphrase'

handler = APIRequest(timeout=4)

dispute = Dispute()

print(dispute.handler)
print(mangopay.get_default_handler())

print(handler)

As you can see after executing it, it will output something like that :

<mangopay.api.APIRequest object at 0x7f4deae39b70>                                                                                                                     
<mangopay.api.APIRequest object at 0x7f4deae39b70>                                                                    
<mangopay.api.APIRequest object at 0x7f4def926d68> 

By looking at the references of the objects, we can easily understand that the resource is using the memoized handler and not the custom one.

As a consequence, defining things like sandbox, timeout, proxies, storage_strategy or anything in the APIRequest is useless if the custom handler doesn't override the default one.

@BboyKeen BboyKeen changed the title Handler misbehaviour APIRequest Handler misbehaviour Oct 12, 2017
@BboyKeen
Copy link
Author

I would add that @Changaco already made a remark in #96 but you didn't really pay attention I think

I second this issue. The way this library manages its default handler is convoluted and broken. I had to use a monkeypatch like this:

handler = mangopay.APIRequest(client_id=…, passphrase=…, sandbox=…)
mangopay.get_default_handler = mangopay.base.get_default_handler =
mangopay.query.get_default_handler = lambda: handler

@mickaelpois
Copy link
Contributor

Hello @BboyKeen,

Thanks for your remarks about our SDK.
We will analyse your technical proposal and make you a quick return about it.
Depending on its complexity, we will add it on our short-term roadmap or long-term one.

Mickaël

@ecridge
Copy link

ecridge commented Dec 23, 2017

I second this. The handler doesn’t make any sense:

  • If you try to instantiate the handler globally as suggested by the docs (@BboyKeen’s first snippet), the result is completely ignored in favour of the default handler.

  • If you try to pass the handler you instantiated into one of the model methods (e.g. natural_user.save(handler)), it is also ignored in favour of the default handler.

So (as far as I can tell), the only way to set the sandbox option is on mangopay.sandbox directly, and there is no way to set proxies, timeout or storage_strategy?

@BboyKeen
Copy link
Author

@joecridge You should try to use the snippet in my comment to override the handler. This way, you'll be able to set proxies, timeout and so on.

@ecridge
Copy link

ecridge commented Dec 23, 2017

@BboyKeen yeah I’ve used that as a workaround, thanks. Ideally, though, it would be nice if the documentation was updated to match the implementation, or vice versa.

@ecridge
Copy link

ecridge commented Feb 20, 2018

Just a note to add that the above snippets don’t work on their own, because the handler always falls back to the sandbox constant from the SDK’s __init__.py:

# From the mangopay.api module:
class APIRequest(object):
    def __init__(self, ... , sandbox=None, ... ):
        if sandbox or mangopay.sandbox:  # <--- !!!
            self.api_url = api_sandbox_url or mangopay.api_sandbox_url
        else:
            self.api_url = api_url or mangopay.api_url

Since mangopay.sandbox is set to True in your source, passing sandbox=False to the APIRequest constructor has no effect.

So to use the production environment and specify any other handler settings, you need to do something like this:

import mangopay

# Set sandbox.
mangopay.sandbox = False

# Set your other settings.
_handler = mangopay.APIRequest(<settings excluding sandbox>)

def get_default_handler():
    return _handler

# Monkeypatch `get_default_handler` everywhere.
mangopay.get_default_handler = get_default_handler
mangopay.base.get_default_handler = get_default_handler
mangopay.query.get_default_handler = get_default_handler

This is just a combination of @BboyKeen’s code from above and @chocopoche’s code from #96, but hopefully seeing them in combination might be of help to the next person who stumbles upon this issue. 🙂

@mangomaxoasis
Copy link
Contributor

Changes added in latest release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants