bot_data as global memory (V12 version of #1322)

[Bibo-Joshi]

This PR implements the global memory proposed in #1318 by providing a dictionary bot_data in addition to user_data and chat_data in a V12 compatible way (i.e. using context based callbacks, replacing #1322).

Integration of bot_data in the Dispatcher class is provided.
Since the bot_data is a global memory, it will always be added to a CallbackContext (i.e. every call

callback_context = CallbackContext.from_...(disptacher)

will add bot_data to callback_context).

Integration in persistence is also provided. Note, that in update_bot_data, the data is updated by

self.bot_data = data.copy()

in contrast to the current implentation for user/chat_data

self.user/chat_data[chat_id] = data

This is due to problems as described in #1297. However, this does not fix #1297 for bot_data. Whan a good way to handle those problems is found, it should be implemented for bot_data (e.g. usage of deepcopy as proposed in #1301)

In DictPersistence, I just used json.loads() to decode serialized data. For user_data and chat_data, a function from the helpers module is used to allow integer keys. imo, this is not really necessary and may lead to problems (e.g. when using '3' as a key, that will be decoded as 3), so I left it out. If I just didn't get the point of it, I will gladly add this.

Also, I've adjusted the test routines. Since I'm not familiar with the pytest module, please have a close look at those.

[jsmnbom]

So far I've only done a quick look over, but it looks really good!
I added a few comments/questions that I'd like you to answer :)

And I think you did a good job with the pytest stuff :D

[jsmnbom]

In terms of

In DictPersistence, I just used json.loads() to decode serialized data. For user_data and chat_data, a function from the helpers module is used to allow integer keys. imo, this is not really necessary and may lead to problems (e.g. when using '3' as a key, that will be decoded as 3), so I left it out. If I just didn't get the point of it, I will gladly add this.

I think the reason we do that, is that it's a user_data and chat_data is a defaultdict, with the ids (int) as the key.
So the way you've done here is good :)

Oh and in terms of failed tests.. Most of them you can ignore - we're working on fixing them, but progress is slow :)
It would be lovely if you could fix the flake8 errors (even though I realise that you're probably not the cause, but we changed the configs and you're the first to touch the file since then.)

[Bibo-Joshi]

Thanks for the comments, @jsmnbom!

In terms of

In DictPersistence, I just used json.loads() to decode serialized data. For user_data and chat_data, a function from the helpers module is used to allow integer keys. imo, this is not really necessary and may lead to problems (e.g. when using '3' as a key, that will be decoded as 3), so I left it out. If I just didn't get the point of it, I will gladly add this.

I think the reason we do that, is that it's a user_data and chat_data is a defaultdict, with the ids (int) as the key.
So the way you've done here is good :)

Yeah, but something like user_data[user_id][123] = 'foo' is allowed as well.

It would be lovely if you could fix the flake8 errors (even though I realise that you're probably not the cause, but we changed the configs and you're the first to touch the file since then.)

No problem. I took the liberty and fixed it for commandhandler.py, too, which I didn't touch otherwise.

[jsmnbom]

Please see my comments in #1301. Some of them apply here too. TLDR: not merging for inclusion in V12 beta just yet since I'm not 100% sure of the internals of persistence.
Also we seem to have a bunch of commits - probably cause the flake8 stuff got fixed elsewhere - sorry about that :)

[Eldinnie]

I would like to pipe in here. I think this feature is a bit overdone. I don;t see why adding a simple dict to the bot class wouldn't serve the purpose looked for here. I know it ties in with persistence in this way, but I;m not a big fan of introducing new variable to the persistence. It's mainly created to make the ones we already had persistent.

I Appreciate the time you put in to help us here, but if it;s up to me I would like to not implement this feature...

[Bibo-Joshi]

While I'd love to have this for some of my bots, the decision clearly is up to you :D
Just resolving conflicts in case you decide to merge.

[Bibo-Joshi]

Added chances according to #1312.
Needs to be revisited, if #1462 is merged.

[Poolitzer]

I see no reason for this. Providing chat/user data as a (temporary) storage is great and often used, if someone wants a global memory, they should setup a database.

[Bibo-Joshi]

Maybe I have a different view on this since I always used user_data and chat_data in combination with Persistence and never knew them without …

IMHO, BasePersistence gives a neat way to give users access to persistence services without having to worry about the details and having to inherit from telegram.Bot to add a dict attribute is a bit tiresome …
If we had a Persistence class that works with a proper database (SQL or something), would you feel different about bot_data being a front end to that?

But again, this is my personal opinion and I'm not as experienced as most of @python-telegram-bot/maintainers. I have working code, still I'd love to have this feature natively.

[Poolitzer]

Yes, I would feel way better. But I would also say that this is out of the scope of this project.

Normally, you would try to optimize how you build the database around your bot. Giving users a general interface would feel weird/wrong to me.

I am the counterpart to you. I used chat/user data years before persistence was a thing and would never use it as a permanent storage. Temporary, great. But for everything really persistent, I use a real database. Maybe Im just too conservative though.

[Bibo-Joshi]

Brought this PR up to date.

If merged, the this wiki page and this one need to be updated.

[Eldinnie]

@Bibo-Joshi also the persistence page in wiki will need to be updated right?