Skip to content

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Jun 11, 2020

Closes #1992

I went for the __new__ approach, but that's up for discussion.

Handling arbitrary collections that may contain bots is really not easy. For example, how would one replace a non-mutable Mapping subclass that contains a bot with an corresponding object, where the bot is replaced, if one does not know how it was initialized? Therefore I'm currently only handling the built-in types ((default)dict, list, tuple, (frozen)set). IMO that should suffice, as the more important use case is a bot as ordinary attribute (which is far easier to handle via __dict__ and __slots__). Still, we'll need a proper documentation for that, if that's approved.

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

What about putting a warning in the docstring, that we replace tokens when they change in pickling, but that this might result in Chat not found errors?

Code looks good!

@Bibo-Joshi
Copy link
Member Author

py3.7 fail unrelated and codacy has problems with the v13 branch (we'll see any problems in #1982, though). Merging.

@Bibo-Joshi Bibo-Joshi merged commit 21f6fbf into v13 Jul 13, 2020
@Bibo-Joshi Bibo-Joshi deleted the bot-persistence branch July 13, 2020 19:52
Bibo-Joshi added a commit that referenced this pull request Jul 14, 2020
* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation
Bibo-Joshi added a commit that referenced this pull request Jul 19, 2020
* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation
Bibo-Joshi added a commit that referenced this pull request Jul 19, 2020
* Refactor handling of `default_quote`

* Make it a breaking change

* Pickle a bots defaults

* Temporarily enable tests for the v13 branch

* Temporarily enable tests for the v13 branch

* Refactor handling of kwargs in Bot methods (#1924)

* Unify kwargs handling in Bot methods

* Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class

* Fix test_official

* Update get_file methods

* Refactor JobQueue (#1981)

* First go on refactoring JobQueue

* Temporarily enable tests for the v13 branch

* Work on tests

* Temporarily enable tests for the v13 branch

* Increase coverage

* Remove JobQueue.tick()

# Was intended for interal use anyways
# Fixes tests

* Address review

* Temporarily enable tests for the v13 branch

* Address review

* Dispatch errors

* Fix handling of job_kwargs

* Remove possibility to pass a Bot to JobQueue

* Refactor persistence of Bot instances (#1994)

* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation

* Add warning to Updater for passing both defaults and bot

* Address review

* Fix test
Bibo-Joshi added a commit that referenced this pull request Aug 13, 2020
* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation
Bibo-Joshi added a commit that referenced this pull request Aug 13, 2020
* Refactor handling of `default_quote`

* Make it a breaking change

* Pickle a bots defaults

* Temporarily enable tests for the v13 branch

* Temporarily enable tests for the v13 branch

* Refactor handling of kwargs in Bot methods (#1924)

* Unify kwargs handling in Bot methods

* Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class

* Fix test_official

* Update get_file methods

* Refactor JobQueue (#1981)

* First go on refactoring JobQueue

* Temporarily enable tests for the v13 branch

* Work on tests

* Temporarily enable tests for the v13 branch

* Increase coverage

* Remove JobQueue.tick()

# Was intended for interal use anyways
# Fixes tests

* Address review

* Temporarily enable tests for the v13 branch

* Address review

* Dispatch errors

* Fix handling of job_kwargs

* Remove possibility to pass a Bot to JobQueue

* Refactor persistence of Bot instances (#1994)

* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation

* Add warning to Updater for passing both defaults and bot

* Address review

* Fix test
Bibo-Joshi added a commit that referenced this pull request Aug 16, 2020
* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation
Bibo-Joshi added a commit that referenced this pull request Aug 16, 2020
* Refactor handling of `default_quote`

* Make it a breaking change

* Pickle a bots defaults

* Temporarily enable tests for the v13 branch

* Temporarily enable tests for the v13 branch

* Refactor handling of kwargs in Bot methods (#1924)

* Unify kwargs handling in Bot methods

* Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class

* Fix test_official

* Update get_file methods

* Refactor JobQueue (#1981)

* First go on refactoring JobQueue

* Temporarily enable tests for the v13 branch

* Work on tests

* Temporarily enable tests for the v13 branch

* Increase coverage

* Remove JobQueue.tick()

# Was intended for interal use anyways
# Fixes tests

* Address review

* Temporarily enable tests for the v13 branch

* Address review

* Dispatch errors

* Fix handling of job_kwargs

* Remove possibility to pass a Bot to JobQueue

* Refactor persistence of Bot instances (#1994)

* Refactor persistence of bots

* User BP.set_bot in Dispatcher

* Temporarily enable tests for the v13 branch

* Add documentation

* Add warning to Updater for passing both defaults and bot

* Address review

* Fix test
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020
@Bibo-Joshi Bibo-Joshi added 🔌 bug pr description: bug 🔌 enhancement pr description: enhancement and removed bug 🐛 labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔌 bug pr description: bug 🔌 enhancement pr description: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants