Support for multiple tokens #2

Activity

1. saleweaver commented on Jul 17, 2025

   

   saleweaver

   on Jul 17, 2025

   Owner

   More actions

   Hi @ZejdCicak ,

   thanks for raising this - it's actually an overview on my side, usually the cache key should be a hash of the refresh token, not a static "token"... This library is built upon my other one, https://github.com/saleweaver/python-amazon-sp-api, where this is implemented as this already.

   If you want to port it over, or create your own, you're very welcome to do so - I'll happily merge it.

   Here's the implementation in the other lib
   https://github.com/saleweaver/python-amazon-sp-api/blob/master/sp_api/auth/access_token_client.py
2. ZejdCicak commented on Jul 17, 2025

   

   ZejdCicak

   on Jul 17, 2025

   Author

   More actions

   Thank you, I've provided a solution here. Please let me know what you think!
3. saleweaver commented on Jul 17, 2025

   

   saleweaver

   on Jul 17, 2025

   Owner

   More actions

   Thanks for that. I've only seen it on mobile, but I think now we're recreating the cache whenever a new client gets created (unless you explicitly pass an existing cache).

   I like the cache service, nice one. Maybe we should make it a singleton?

   Copilot recommended sth like this, I'll have a look tomorrow as well:

   To address the caching limitations while leveraging the service provided in the pull request, here's an alternative approach that ensures shared caching across multiple client instances:

   Alternative Approach

   1. Singleton Cache Service:
      Use a singleton pattern for the CacheService to ensure all client instances share the same cache instance. For example:

      from threading import Lock
      
      class CacheServiceSingleton:
          _instance = None
          _lock = Lock()
      
          def __new__(cls, *args, **kwargs):
              with cls._lock:
                  if not cls._instance:
                      cls._instance = CacheService(*args, **kwargs)
              return cls._instance

   2. Modify Client Initialization:
      Update the client to use this singleton CacheService by default if no explicit cache service is provided:

      class Client:
          def __init__(self, cache_service=None):
              self.cache_service = cache_service or CacheServiceSingleton()
              # Other initialization code...

   3. Shared Tokens:
      With the singleton approach, all clients will share the same cache, ensuring tokens are reused across clients, reducing redundant retrievals.

   4. Custom Cache Configuration:
      If needed, allow customizing the cache configuration for the singleton:

      class CacheServiceSingleton:
          _instance = None
          _lock = Lock()
      
          @classmethod
          def configure(cls, *args, **kwargs):
              with cls._lock:
                  cls._instance = CacheService(*args, **kwargs)
      
          def __new__(cls, *args, **kwargs):
              with cls._lock:
                  if not cls._instance:
                      cls._instance = CacheService(*args, **kwargs)
              return cls._instance

      This way, the cache configuration can be set globally before any client instances are created.

   ---

   This approach keeps the flexibility introduced by the abstract CacheService while maintaining the benefits of a shared, top-level cache for tokens. Would you like help implementing or refining this solution further?
4. ZejdCicak commented on Jul 17, 2025

   

   ZejdCicak

   on Jul 17, 2025

   Last edited by ZejdCicak

   Author

   More actions

   I've done it this way due to having client id in constructor of the CacheService so we can build unique key for each client, I wasn't sure how would you feel about adding client_id to get_key contract.

   If you think adding client_id to get_key contract (which IMO wouldn't be a bad idea overall) is okay, I would be happy to change it to singleton approach.
5. ZejdCicak commented on Jul 17, 2025

   

   ZejdCicak

   on Jul 17, 2025

   Last edited by ZejdCicak

   Author

   More actions

   I have prepared changes like this, I just didn't push them yet in case you have another idea about client id. The contract looks like this now:

   @abstractmethod
   def get_key(self, client_id: str) -> str:
      """Generate a cache."""
       pass

   While CacheServiceSingleton looks like this:

   class CacheServiceSingleton(AbstractCacheService):
       _instance = None
       _lock = Lock()
   
       def __new__(cls):
           # Check if an instance already exists without acquiring the lock
           if cls._instance is None:
               with cls._lock:
                   if cls._instance is None:
                       cls._instance = super().__new__(cls)
                       cls._instance._cache = TTLCache(maxsize=10, ttl=3600)
   
           return cls._instance
6. saleweaver commented on Jul 18, 2025

   

   saleweaver

   on Jul 18, 2025

   Owner

   More actions

   Let's try that..

   👍React with 👍1Reacted by ZejdCicak
7. ZejdCicak commented on Jul 18, 2025

   

   ZejdCicak

   on Jul 18, 2025

   Author

   More actions

   Gotcha! It's in the PR now.
8. ZejdCicak commented on Jul 21, 2025

   

   ZejdCicak

   on Jul 21, 2025

   Last edited by ZejdCicak

   Author

   More actions

   @saleweaver Sorry to ping you again, any update on this and another PR I submitted? I would really appreciate if you could check them since I'm in a bit of a hurry, and this is blocking me, and I would rather contribute to your project, instead of forking it. Thank you!