Skip to content

Fix bug: OverflowError: mktime argument out of range#94

Open
samwellzuk wants to merge 1 commit into
sibson:masterfrom
samwellzuk:master
Open

Fix bug: OverflowError: mktime argument out of range#94
samwellzuk wants to merge 1 commit into
sibson:masterfrom
samwellzuk:master

Conversation

@samwellzuk

Copy link
Copy Markdown

The variable use to call is_due->maybe_make_aware->is_navie->utcoffset->_isdst->_time.mktime

, raise a exception on windows: OverflowError: mktime argument out of range

The min timestamp is 1970-1-1, considering timezone, if you are in east+8, 1970-1-1+8 turn into 0,
so the best safe value is 1970-1-2

…t->_isdst->_time.mktime

, raise a exception on windows: OverflowError: mktime argument out of range

The min timestamp is 1970-1-1, considering timezone, if you are in east+8, 1970-1-1+8 turn into 0,
so the best safe value is 1970-1-2

@sibson sibson left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Seems reasonable. Could you take a look at the failing tests?

Comment thread redbeat/schedulers.py

return self.schedule.is_due(self.last_run_at or
datetime(MINYEAR, 1, 1, tzinfo=self.schedule.tz))
datetime(1970, 1, 2, tzinfo=self.schedule.tz))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Literal constants always set off alarms in my head but I don't see a good way to eliminate them in this case.

@sibson sibson added the question label Apr 3, 2019
@sibson

sibson commented Aug 20, 2019

Copy link
Copy Markdown
Owner

This has merge conflicts, once resolved I'll re-review.

@sibson

sibson commented Feb 16, 2025

Copy link
Copy Markdown
Owner

Conflicts need resolving and tests passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants