Skip to content

Support for tz-aware datetime.datetime objects #1451

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.

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

Closed

Conversation

torresjrjr
Copy link

Original issue

Issue: #1409
version: 12.0.0b1

Changes

The ._put() method can now accept utc_offset-aware/timezone-aware datetime.datetime objects. This helps bots to be hosted on a server with any timezone without complications.

To convert datetimes to UTC, a "UTC" subclass of the datetime.tzinfo class was added, and used in ._put().

Other options considered

  • Changing all the .now() comparisons in ._put() to .utcnow(). This was not chosen, since it would cause a major compatibility issue. Bots would have to be rewritten to pass in UTC datetimes, instead of local (computer time) datetimes, into the jobqueue.run_once and other methods. This would likely be unwanted behaivour.

## Original issue ##
Issue: python-telegram-bot#1409 
version: 12.0.0b1

## Changes ##
The `._put()` method can now accept utc_offset-aware/timezone-aware `datetime.datetime` objects. This helps bots to be hosted on a server with any timezone without complications.

To convert datetimes to UTC, a "UTC" subclass of the `datetime.tzinfo` class was added, and used in `._put()`.

## Other options considered ##
- Changing all the `.now()` comparisons in `._put()` to `.utcnow()`. This was not chosen, since it would cause a major compatibility issue. Bots would have to be rewritten to pass in UTC datetimes, instead of local (computer time) datetimes, into the jobqueue.run_once and other methods. This would likely be unwanted behaivour.
@Eldinnie
Copy link
Member

Hi @torresjrjr
Thanks for making this PR, and our excuse for not responding sooner.
I like the addition and the idea behind it, but I would like to see you add some tests reflecting the changes.

Could you add those please?

@Eldinnie Eldinnie added the 📋 pending-reply work status: pending-reply label Aug 23, 2019
@tsnoam
Copy link
Member

tsnoam commented Aug 23, 2019

Actually, I like to open this for discussion with the other maintainers.
IMHO, servers should run on UTC . Timezones add so many questions and problems.
My vote is not to add this code to the library.

Copy link
Member

@tsnoam the point they make in the issue, is that sometimes you want to use run_daily for the user-aware timezone. This PR makes it possible to say:
Run daily on timezone +5 and it will convert to UTC to put in the job_queue.
It assumes the server on UTC and makes it easier to add jobs for all timezones without the hassle of calculating the timedelta manually or program it in every bot

@tsnoam
Copy link
Member

tsnoam commented Aug 23, 2019

Well, that's a subset of what a timezone is.
A timezone can also be "Asia/Jerusalem". Now go and map between summer and winter clock. Or, if you're on the edge of switching between them, switch.
Not to mention countries which change their decision when to move between summer & winter clock every year.
I prefer not to "put a healthy head inside a sick bed".

@torresjrjr
Copy link
Author

Actually, I'm not so invested in this PR as much, since this was an attempt at a solution to a problem I later solved. I wanted to "help my bot to be hosted on a server with any timezone without complications" but I ended up simply adding an extra step in my code.

I created a timedelta object of the time from now() to the datetime, and submitted that to my job creations. It's a quick solution others might find useful :) and could be used in the final PR in the .put() method.

I should also stress that I'm a python noob/intermediate. I find PRs scary. So please, take my idea and do what you will. Thanks.

@Eldinnie
Copy link
Member

We were debating whether this should be a here, in helpers or in the programmers hands. I guess you helped us answering that. I'm closing this PR.

@Eldinnie Eldinnie closed this Aug 23, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📋 pending-reply work status: pending-reply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants