Skip to content

Pass context of last valid update to timeout handlers #1826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Bibo-Joshi merged 2 commits into from
Apr 10, 2020
Merged

Pass context of last valid update to timeout handlers #1826

Bibo-Joshi merged 2 commits into from
Apr 10, 2020

Conversation

@Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Mar 10, 2020

Closes #1823

Also added a test case that makes sure that the last update & context are the same as the ones in the timeout handler.

@Bibo-Joshi Bibo-Joshi added the 📋 pending-review work status: pending-review label Mar 10, 2020
@Bibo-Joshi Bibo-Joshi added this to the 12.5 milestone Mar 10, 2020
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Mar 10, 2020

test_set_game_score scores again …

@artemrys
Copy link
Contributor

artemrys commented Mar 18, 2020

@Bibo-Joshi regarding your comment about test_set_game_score test I have prepared a draft PR with a possible workaround for this issue. PTAL https://github.com/artemrys/python-telegram-bot/commit/3734b01898b724f41f20f27281b4b643160b0d19 It works for me locally.
Does it worth to create a separate bug for this issue and start discussing it there?

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Mar 18, 2020

@Bibo-Joshi regarding your comment about test_set_game_score test I have prepared a draft PR with a possible workaround for this issue. PTAL artemrys@3734b01 It works for me locally.
Does it worth to create a separate bug for this issue and start discussing it there?

The failing tests are a more general issue, see also #1759. So, yes, please go ahead and pr, so we can see, if the reordering also works in the CI. Please make sure to comment, why the reorder should help.

This was referenced Mar 19, 2020
Closed
Merged
@Bibo-Joshi Bibo-Joshi mentioned this pull request Mar 27, 2020
Closed
@Bibo-Joshi
Merge branch 'master' into fix-1823
38c24a8 
# Conflicts:
#	telegram/ext/conversationhandler.py
Poolitzer
Poolitzer reviewed Apr 10, 2020
View reviewed changes
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.

LGTM. Please merge, its a bug.

@Bibo-Joshi Bibo-Joshi merged commit bdf0cb9 into master Apr 10, 2020
@Bibo-Joshi Bibo-Joshi deleted the fix-1823 branch April 10, 2020 11:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Poolitzer Poolitzer Poolitzer left review comments

Assignees

No one assigned

Labels

📋 pending-review work status: pending-review

Projects

None yet

Milestone

12.6

Development

Successfully merging this pull request may close these issues.

[BUG] context.user_data saved in a conversation is lost upon timeout.

4 participants

@Bibo-Joshi @artemrys @Poolitzer