tingbot / tingbot-python

🔩 Python APIs to write apps for Tingbot
Other
18 stars 9 forks source link

Settings #22

Closed furbrain closed 8 years ago

furbrain commented 8 years ago

This pull request implements persistent storage for tingbot apps. It essentially gives you access to a dict-like object (tingbot.config), which will read default_settings.json, gui_settings.json, and local_settings.json (in increasing priority) default_settings.json is intended to be created by the app developer and otherwise left unchanged gui_settings.json is in place so that the app can be configured by the end-user in Tide (or a.n.other IDE) local_settings.json will contain data stored directly from the app.

I have also put in some test code.

There is a known bug in that if you assign to a subdictionary or list , then it will not automatically save this. I can't think of a good way to fix this, but have documented it in the documentation.

Usage

import tingbot

#store an item
tingbot.config['favourite_colour'] = 'red'

#local_settings.json on disk now contains: {"favourite_colour":"red"}

#retrieve an item
tingbot.screen.fill(tingbot.config['favourite_colour'])
joerick commented 8 years ago

Thanks @furbrain, this is a great starting point, I think. Also, tests! 😀 Fantastic.

What are your thoughts about supplying metadata (displayname, type) with the default settings - so a GUI could draw a form with the data? It was something I suggested in #20, maybe you disagree? Would like to hear your thoughts.

joerick commented 8 years ago

Here's what I'm thinking

API

label_color = tingbot.app.settings['favourite_color']

Or, if you've done the * import

label_color = app.settings['favourite_color']

(The app object is also somewhere we could some app introspection, such as icon, version number etc.)

Also, I'd be in favour of renaming gui_settings.json to just settings.json, since it doesn't have anything to do with a GUI at the moment, and can be used with a text editor fine.

I'll have a think about the settings schema metadata, but we can leave that as a follow-on task for now.

joerick commented 8 years ago

You happy to make the above changes @furbrain? Otherwise I'm happy to pick this up and make the changes. :)

furbrain commented 8 years ago

I'll make those changes this evening - I'll move settings.py to app.py, create an App object, and attach an instance of my SettingsDict to it.

joerick commented 8 years ago

Sounds good! Thanks.

The module app.py might have to be called something else to make an instance of App object available at tingbot.app

furbrain commented 8 years ago

good point - I'll pick something else. meta.py?

joerick commented 8 years ago

How about tingapp.py? The class might also be useful to tbtool and the springboard as a way to access metadata on an app at a given path :)

furbrain commented 8 years ago

Ok, I've made those changes - settings.py is now tingapp.py, and contains an extra class TingApp. This can be initialised without any arguments and it will allow introspection of the current running app. Alternatively you can initialise it with a path to an app, and you can introspect (and change the app-local settings) for that app

joerick commented 8 years ago

How about preventing dicts from being stored in settings? I'm thinking that an easy way to add the schema would be to 'upgrade' to a dictionary instead of a value

e.g. changing default_settings.json

{
  "text_color": "a3ddff"
}

to

{
  "text_color": {
    "type": "color",
    "display_name": "Text colour",
    "value": "a3ddff"
  }
}

...and it would solve the confusion from the mutable dictionary (having to call save()). UNLESS we can think of a compelling use case for storing a dictionary in settings?

furbrain commented 8 years ago

Use case; just off the top of my head: e-mail checker - just flags up whether you have new mail - could poll several different email accounts e.g:

{
  "accounts": {
    "google": {
      "username":"LarryBrin23",
      "password":"Passw0rd" 
    },
    "hotmail": {
      "username":"SpamLord",
      "password":"spamspamspammityspam"
    }
  }
}

There are probably plenty more potential use cases. Python dicts are one of the most powerful features of the language and I am a bit reluctant to remove this capability from the settings module. The problem with having to call save() would also apply to any lists, so banning dict would not particularly address this issue. I think my preferred way of addressing where to store the schema would be in either a separate file like schema.json or possibly in default_settings.json but under a key of "_schema"

joerick commented 8 years ago

I think the above email feature would be better served with a list, personally:

{
  "accounts": [
    {
      "name": "google",
      "username":"LarryBrin23",
      "password":"Passw0rd" 
    },
    {
      "name": "hotmail",
      "username":"SpamLord",
      "password":"spamspamspammityspam"
    }
  ]
}

Happy to drop the idea if you really disagree though. The only reason I'm thinking about it is that I don't think I've needed to store a dictionary in settings before, except when I'm using settings as persistent storage for an app. But really I'm coming at it from the syntax of default_settings.json, which is maybe wrong, because there are hundreds of ways to do that schema.

Ok, I've persuaded myself, let's allow dictionaries.

We could solve the save() confusion by-

Of these I'd favour the second, since it preserves the programmer intent. What do you think?

joerick commented 8 years ago

Merged!