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

Proposal: Add option to pass in state for each API request #128

Open
Cidan opened this issue Oct 22, 2021 · 5 comments
Open

Proposal: Add option to pass in state for each API request #128

Cidan opened this issue Oct 22, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@Cidan
Copy link

Cidan commented Oct 22, 2021

First of all, thank you for your amazing work on this library, it's great!

The library in general is written with single-user use cases in mind and doesn't work well in a multi-tenant system, i.e. a microservice which handles API calls for multiple users, bot id's, etc. This is because the state of all authentication mechanisms is stored within the instantiated object it self.

One option would be to create a new object per user, but this creates overhead, eliminates the possibility of some design patterns (i.e. singleton instantiation), and would require a tighter coupling of the driver to the authentication source.

Ideally, all authentication would be stateless and be passed in per call. At the very minimum, user and application tokens being passed in per API call would provide sufficient abstraction insofar that multiple users can be used on the same bot, concurrently, without having to create a new object for every user.

I'm curious to hear what you think @nicklaw5 about this proposal.

Thanks!

@Cidan Cidan changed the title Proposal: Add option to remove or pass in state from library Proposal: Add option to pass in state for each API request Oct 22, 2021
@nicklaw5
Copy link
Owner

So I can understand your problem better, are you able to provide an example what you're trying to achieve with this library in its current state and the issues you're having?

@Cidan
Copy link
Author

Cidan commented Oct 23, 2021

Absolutely!

So what I've been working on is a decoupled system in which outbound Twitch API calls are processed in their own stand alone microservice via RPC's. Right now, if I want to call a Twitch API using this library, I would need to have some sort of locking mechanism to ensure concurrent use of the library isn't done across multiple users.

For example, let's say I have 200 users with 200 user tokens that have been granted permission to a bot. Every time I want to call the Twitch API for something, I would send an RPC to the microservice I wrote that contains your Twitch library. This service would then call your Twitch library with the end user credentials by first calling SetUserAccessToken method on the client from your library.

The problem here is, I can't have multiple tokens in use concurrently across the same instantiation of this library, because the user token it self is stored as state in the library. I thus need to either implement locking so that only one user can use a client at a time, i.e. sync.Lock or client pooling.

Both solutions are undesirable as the API is stateless by nature -- there should be no need to lock or pool when the lowest level of the API (i.e. the http calls) is stateless.

What I propose is something like:

resp, err := client.GetUsers(
    &helix.UsersParams{
        IDs:    []string{"26301881", "18074328"},
        Logins: []string{"summit1g", "lirik"},
    },
    &helix.AuthOptions{
        UserAccessToken: "some-access-token",
    },
)

Does that clarify the problem?

Thanks!

@nicklaw5
Copy link
Owner

nicklaw5 commented Nov 9, 2021

I agree that is generally a good idea on the surface, but without diving in and having a deeper look I'm not sure what changes need to be implemented.

I'm also rather busy at the moment and not sure when I'll be able to have a look at this.

@Cidan
Copy link
Author

Cidan commented Nov 9, 2021

I'll take a look and outline what changes would need to be done in order to get this in place. At the very least, we should be able to measure the level of effort here.

@graytonio
Copy link

I'm running into similar issues with wanting to have multitenant applications has this feature been abandoned?

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

No branches or pull requests

3 participants