Skip to content

Conversation

@Eldinnie
Copy link
Member

This makes a deepcopy of the user_data and chat_data dict as suggested by @Bibo-Joshi
Fixes #1297

This makes a deepcopy of the user_data and chat_data dict as suggested by @Bibo-Joshi
Copy link
Member

jsmnbom commented Feb 13, 2019

@Eldinnie So we are fine with the performance penalty "issue" as raised by @Bibo-Joshi in #1301 then?

@jsmnbom
Copy link
Member

jsmnbom commented Feb 13, 2019

I'm personally fine with it. If people were gonna store massive amounts of data, they would use a proper library/database for it instead I think.

@Eldinnie
Copy link
Member Author

I think we have no choice, and I think the performance penalty will be minimal. Also the fix proposed in #1301 (oops, missed that one before stating on this) means it;s a choice between deepcopy() and unneeded writes to disk (which are even more time consuming) so I guess just deepcopying is fine

Copy link
Member

@jsmnbom jsmnbom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, LGTM then.

@jsmnbom jsmnbom added this to the V12 milestone Feb 13, 2019
@jsmnbom
Copy link
Member

jsmnbom commented Feb 13, 2019

CI fail unrelated, will fix on master.

@jsmnbom jsmnbom merged commit 7e2dbdd into V12 Feb 13, 2019
@Eldinnie Eldinnie deleted the fix_1297 branch February 14, 2019 12:48
Eldinnie added a commit that referenced this pull request Feb 2, 2020
* Update AUTHORS.rst

* Update AUTHORS.rst

* Add bot_data to CallbackContext as global memory

* Minor fixes in docstrings

* Incorp. req. changes, Flake8 Fixes

* Persist before stop

* Fix CI errors

* Implement #1342 for bot_data

* Add check pickle_persistence_only_bot similar to #1462

* Fix test_persistence

* Try dispatching error before logging it

* Fix test

Co-authored-by: Eldinnie <Eldinnie@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants