Cleanup config

[mattman107]

The goal of this PR is to standardize how config read/writes are done. Before a new case had to be written per value to be save in the config file. By standardizing the names of the keys and using a dictionary to store our variables in ram, we can loop through when reading and writing the config, greatly decreasing the complexity/length of the code.

This was a big update so might want to go through this with a fine tooth comb to make sure I didn't mess anything up.

[h-quer]

This looks great, thanks, that does make the config much neater!

Minor things I've noticed:

• I've been using underscores as separater instead of upper case, e.g. client_id instead of clientId - ultimately either works, but for consistency, it makes sense to stick to one convention, in this case config_dictionary instead of configDictionary
• So far, I've used only functions and no classes, mainly since the latter weren't really needed. I've been thinking about whether an object oriented approach makes sense but since it's a rather small projects, I don't think it's necessary for now. Again, for consistency, I'd avoid the configUpdate class for now and simply use a config_update function - or is there a specific reason you chose the class?

[mattman107]

1. Got it. Next time I get a chance I will clean up my variable names.
2. So the configUpdate class was something I made so there wasn't a million parameters in the update_config function. So when you need to update the config file you can instantiate a configUpdate object that will contain a section, key, and value that corresponds with the section, key, and value in the config file/ config dictionary. Here is an example of this in the Slaanesh_IGDB.py file:

def refresh_igdb_token():
    now = dt.datetime.now()
    url = 'https://id.twitch.tv/oauth2/token'
    params = {'client_id': config.configDictionary['igdb']['client_id'],
              'client_secret': config.configDictionary['igdb']['client_secret'],
              'grant_type': 'client_credentials'}
    response = requests.post(url, json=params)
    json_response = json.loads(response.text)
    dataframe_response = pd.json_normalize(json_response)
    expiry_timestamp = now + dt.timedelta(seconds=int(dataframe_response['expires_in'][0])) - dt.timedelta(days=2)
    expiry_string = dt.datetime.strftime(expiry_timestamp, "%Y-%m-%d %H:%M:%S")
    config.update_config([
                            config.configUpdate('igdb','auth_token', dataframe_response['access_token'][0]),
                            config.configUpdate('igdb','token_timestamp', expiry_string)
                        ])

On the last line of this function we call the update_config function and pass it a list with two instances of the configUpdate class. So the big advantage over this method is that we don't need a new parameter every time we add a new section/key to the config file and it is easy to tell exactly what a configUpdate object requires to exist.

An alternative to a class with predefined variable names would be tuples. I opted not to do this because it would be messier to pass tuples. We would be hard coding what position in the tuple is the section, or the key, or the value. If someone didn't follow the exact structure the way the update_config function was expecting it wouldn't work right.

So the TLDR; I thought it would be cleaner to predefine a data object that could be used rather than use a list of tuples which could be messier.

[h-quer]

I might be missing something very obvious here but why couldn't this be achieved by a simple function call to edit the config dictionary?

So instead of

config.update_config([
                        config.configUpdate('igdb','auth_token', dataframe_response['access_token'][0]),
                        config.configUpdate('igdb','token_timestamp', expiry_string)
                        ])

that would simply be

config.config_update('igdb','auth_token', dataframe_response['access_token'][0])
config.config_update('igdb','token_timestamp', expiry_string)

With the config_update function checking updating the dictionary entries (and checking whether it's a valid update request)?

[mattman107]

Just two different ways to achieve the same thing IMO. It depends if you would rather look through a list of x amount of things to update or call the function x amount of times.

Using this example let me lay out what I think (feel free to disagree) are the good ways to do this kind of update.

The way I wrote in this PR

As you can see we use a class to manage the three variables. We pass a list of classes to the function and iterate through them. The only point of the class is to give a point of reference for what data needs to be in it. It is essentially a custom data type.

config.update_config([
                        config.configUpdate('igdb','auth_token', dataframe_response['access_token'][0]),
                        config.configUpdate('igdb','token_timestamp', expiry_string)
                        ])

The way you are suggesting

If you are against using a class like configUpdate to standardize the process, then I think what you are suggesting above would be a good alternative.

So the function definition would look like:

    def config_update(section, key, value):

and calling the class would look like what you stated above:

    config.config_update('igdb','auth_token', dataframe_response['access_token'][0])
    config.config_update('igdb','token_timestamp', expiry_string)

A list of tuples

This is very similar to the way I wrote this PR initially, except instead of having a class defining what variable is what we are expecting the data to be in a certain part of the tuple. It would look like this

    config.config_update([
                        ('igdb','auth_token', dataframe_response['access_token'][0]), 
                        ('igdb','token_timestamp', expiry_string)
                        ])

The problem I see with using a tuple is that we are trusting that the first position is always the section, the second position is always the key, and the third position is always the value. It is on us to remember that is how it has to be and there isn't an explicit formula for any developer to know what value needs to go where in the tuple.

Personally I don't think we should use the list of tuples method because there is no official definition of what value goes where. That was why I decided to use a class to give the data we want structure and a formal definition what what it should look like.

Let me know what you think. If you believe we should drop the class I will revise.

[h-quer]

I don't quite see why the version with tuples has any issues that the version with classes doesn't have. In both variants, the values have to be passed in the correct and same order. Function parameters can be named as well (calling them with sample_function(key=A, value=B) for example) - I might be missing something, but I don't see the downside of using functions. So yes, for consistency I think it's better to keep things simple and stick to one programming paradigm and not use classes for now. Thanks ;)