[Feature]: Credentials handling improvement suggestions

[RomeoDespres]

Hi, I think there are a couple issues with the current credential management system. For most of them I'd be happy to implement fixes but I think it is better to first discuss them before coding :)

1. Superfluous token refreshes

The current pattern for API calls is the following:

api_call (e.g. pycmc.artist.metadata())
    utilities.RequestData
        importlib.reload(credentials_manager)
            credentials.Update
                credentials.FetchAccessToken
    utilities.RequestGet

Authentication is reperformed before each API call. Hence all functions in the library would be twice as fast if the token was only refreshed when needed (i.e. Error 401).

2. Rate limit handling

I know #15 already references this but I still mention it here because I think it should happen at the same place than token refreshing. In other words, the automatic handling of error 401 (unauthorized) could also handle error 429 (too many requests).

3. Rigid environment variable flow

I think the current mandatory JSON environment variable is not very user-friendly. First, the only needed input from user is the refresh token; the empty dict structure could automatically be infered. Second, I'd say it would be better if users had an alternative way to connect, cause maybe they don't want to use an environment variable.

To solve this, I would imagine a lazy design that does not authenticate at import time but only on first API call, coupled with a new pycmc.authenticate(refresh_token) function. Authentication would use CMCREDENTIALS by default until the user calls authenticate at some point.

4. Impossibility to manage several Chartmetric accounts

I'd like to develop applications that have several users, each with their own API token (think of a web server doing Chartmetric requests in place of the end-user). The current global variable system does not allow this. The way I would redesign the API would be an object-oriented interface similar to for instance what they do at spotipy:

import pycmc

client = pycmc.ChartmetricClient(refresh_token)
client.artists.metadata(...)
client.search_engine.search(...)

That way the application could handle several clients at a time.

---

I think number 1-3 are not breaking changes and are not that hard to implement. Number 4 is a breaking change; even though the API spirit is the same, everything must refactored into this object-oriented interface. I guess you have the right to do such a change when switching to 1.0 but maybe you don't want to since you must already have a bunch of apps running on the current API...

However, if the amount of required coding frightens you, please note that:

1. I already have similar solutions implemented on my side that I could easily transpose to pycmc.
2. I'd be very happy to spend time on this project because I use the Chartmetric API a lot at my company and I'd appreciate having a stable public library for this rather than coding my custom solutions all the time.

[thinkjrs]

@RomeoDespres Thank you for your thoughtful comments. There are a number of reasons for design decisions as they are and why I avoided a 1.0 release up to this point :-). Agree that we need a rewrite of credentials!

I'm buried in other dev for the next 12-24 hours, however, will have the above reviewed in detail by tomorrow morning.

Also, it seems like we have pretty similar wishes, re: public lib for CM. If you'd like, let's move this offline to chat about 1) plans for lib overall and 2) you can jump on as a maintainer here. I'll shoot you a DM on this!

[thinkjrs]

@RomeoDespres Terribly sorry for ghosting. Moving #15 here (and closing), as I completely agree w/combining rate limiting with this.

Let's also go for breaking changes at this point, as I'm in complete agreement with OOP approach, rather than the current pseudo-functional version we originally hacked together.

I also agree with 2 that it should be done in the same object. I'd like to create an AuthManager class that basically implements something similar to the auth_manager work in the spotipy Spotify class. I think in AuthManager it should a function to check the limit, i.e. check a set of request headers from requests and update and/or log. We can instantiate this AuthManager class in your referenced api client class, re: 4 . Let's use Client, instead of ChartmetricClient, and in the class's init we can instantiate the AuthManager.

My objective is to give the best of both worlds: each client is separately contained, along with its own auth logic and token, plus the auth manager could check for the CMCREDENTIALS environment variable. Down the road I'd like the entire lib to have a CLI interface, but that's for a later day at this point.

For reference, here are the specs from #15 :

The current credentials.py and other auth code don't consider rate limiting, which it should. We should implement a means of taking into account:

Header Name	Description
X-RateLimit-Limit	Number of requests allowed per minute on current plan
X-RateLimit-Remaining	Number of requests remaining in current time period (1 minute)
X-RateLimit-Reset	Time at which rate limit will reset

Let's consider that I'd like to nearly replicate the your client api suggestion in 4, and in particular, as the Client class that instantiates its own AuthManager and will instantiate the items in #29.

[thinkjrs]

Let me know if/what you'd like to work on here. I'm going to update deps and ensure tests pass, then start chipping away at #29. I'll quote the PR here so things are clear!