next_t property is added to Job class #1685
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added new property to Job class - next_t, it will show the datetime when the job will be executed next time. The property is updated during JobQueue._put method, right after job is added to queue. Related to python-telegram-bot#1676
Bibo-Joshi
requested changes
Jan 1, 2020
Member
Bibo-Joshi
left a comment
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the PR! Please see my comments below.
Also, we'll need test cases for the new property. Please have a look at test_jobqueue.py and add test cases that make sure that both the setter and getter for next_t work correctly (especially between calls of repeating jobs).
Test Official is failing because we haven't yet implemented Bot API 4.5 - you can ignore that.
1. Added setter for next_t - now JobQueue doesn't access protected Job._next_t. 2. Fixed Job class docstring. 3. Added test for next_t property. 4. Set next_t to None for run_once jobs that already ran.
Bibo-Joshi
requested changes
Jan 2, 2020
1. next_t setter now can accept datetime type. 2. added test for setting datetime to next_t and added some asserts that check tests results. 3. Also noticed Job.days setter raises ValueError when it's more appropriate to raise TypeError.
1. Changed type of error raised by interval setter from ValueError to TypeError.. 2. Fixed test_warning after changing type of errors in Job.days and Job.interval. 3. Added Number type to next_t setter - now it can accept int too.
Andrej730
force-pushed
the
next_t_branch
branch
2 times, most recently
from
e423447 to
f05fe3f
Compare
Added _UTC and _UtcOffsetTimezone for python 2 compatibility
Andrej730
force-pushed
the
next_t_branch
branch
from
f05fe3f to
8c04d69
Compare
Bibo-Joshi
requested changes
Jan 3, 2020
Andrej730
force-pushed
the
next_t_branch
branch
2 times, most recently
from
2dfcd29 to
56fe5f7
Compare
1. Replaced "datetime.replace tzinfo" with "datetime.astimezone" 2. Moved testing next_t setter to separate test. 3. Changed test_job_next_t_setter so it now uses non UTC timezone.
Andrej730
force-pushed
the
next_t_branch
branch
from
56fe5f7 to
3c63112
Compare
Bibo-Joshi
approved these changes
Jan 3, 2020
Andrej730
force-pushed
the
next_t_branch
branch
3 times, most recently
from
70f352f to
0e4b056
Compare
1. Added option to define Job.tzinfo from run_once (by when.tzinfo) and run_repeating (first.tzinfo) 2. Added test to check that tzinfo is always passed correctly.
Andrej730
force-pushed
the
next_t_branch
branch
from
0e4b056 to
051f4b2
Compare
Contributor
Author
|
Because after this PR |
This was referenced Feb 23, 2020
tsnoam
requested changes
Apr 16, 2020
# Conflicts: # tests/test_jobqueue.py
Member
|
Codacy fails as expected, test fail is unrelated. Merging. Thank you for your contribution, @Andrej730! |
Bibo-Joshi
added a commit
to daviddl9/python-telegram-bot
that referenced
this pull request
May 1, 2020
Bibo-Joshi
added a commit
that referenced
this pull request
May 2, 2020
* added monthly job * removed fold argument * addressed pr comments * addressed pr comments * made changes from pr review * updated comments * clean up code * Update .pre-commit-config.yaml * Minor cleanup * Update according to #1685, minor robustness changes Co-authored-by: Hinrich Mahler <hinrich.mahler@freenet.de>
Bibo-Joshi
added
🔌 enhancement
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added new property to Job class - next_t, it will show the datetime when the job will be executed next time.The property is updated during JobQueue._put method, right after job is added to queue.
Related to #1676