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

The python client is not threadsafe #360

Open
karolyi opened this issue Feb 7, 2025 · 2 comments
Open

The python client is not threadsafe #360

karolyi opened this issue Feb 7, 2025 · 2 comments

Comments

@karolyi
Copy link

karolyi commented Feb 7, 2025

Hello,

Coming from having talked to support, I'm reporting this here.

You use a singleton pattern where you instantiate one instance of every possible api handler. That applies to the HTTP client api as well.

Problem is, applications are often multithreaded (in my case it's UWSGI), and this can result in thread safety issues.

I'm in the process of rewriting my integration to use single instantiated objects instead of said singleton pattern. The reason for this is that I use different API keys/settings for my ECOM and POS integration.

Just wanted to let you know. Not sure if this is documented.

Best Regards,
László

@gcatanese
Copy link
Contributor

Hello @karolyi, thanks for reporting this. You are right in pointing out this implementation flow, I think the library has started very small (checkout), but later new versions have introduced the support of new APIs without considering the design.

We are going to look at the options, maybe mimic what happens in the other libraries.

Do you have any suggestion for us? How the library should make easy to support your use cases (ECOM, POS, etc.)? We'd love to hear your inputs while we consider the redesign. Thank you.

@karolyi
Copy link
Author

karolyi commented Feb 19, 2025

Hey,

I don't think you have much space for improvement here since as far as I understand, you use some kind of an API generator to create the python library itself, no?

I just started importing and instantiating the respective API parts from the submodules (with a separate HTTP client instantiated each time it's needed), and stopped using the main Adyen export it offers. With respect to that, the submodules are organized well enough, kudos for that.

Python itself is super effective in instantiating new objects of classes, with a small memory footprint, if that's your concern.

If it would be on me (and I'd get paid for it), I would just write an entirely newly organized library, but I realize that since you use that API generator for python and other languages as well, you don't want to take on the burden of maintaining something completely separate. As you might have figured, I'm a python guy (part of the old guard) so I'm mostly interested in that.

Also, some typing would be nice to have as well and better implementing program paths so that type checker autodetects wouldn't need to be overridden here and there. My first candidate would be pointing out that the code parts that would return AdyenResult | None | Unknown actually don't, because they will raise within a called function if there is not an AdyenResult arrived. In that case you could just return the Exception and raise in the respective class method, so that the static type checkers would understand. Or you could provide extra typing/interface files (.pyi) that would clarify these for the static type checkers. See this, for example:

Image

I use basedpyright as a static checker and it assesses the list_terminals as it's shown in the screenshot: I actually have to suppress the warning message with the passed comments in the code (very inconvenient), and had to implement the following protocol to properly depict the return codes of that function for my usecase:

class ListTerminalsProtocol(Protocol):
    def __call__(self, query_parameters: dict[str, int]) -> AdyenResult: ...

This is what you can see being assigned.

All in all: don't worry, I can find my ways around it and implement an effective use of the library, I figure others can do that as well. Maybe pointing the thread safety problem out somewhere visible would be a good idea.

Nonetheless, let me know if I can help. I would be happy to, provided I'll have the time for it.

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

No branches or pull requests

2 participants