Skip to content
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

timestamps: fix second calculation between datetime objects #18

Merged
merged 5 commits into from
Aug 25, 2015

Conversation

saaros
Copy link
Contributor

@saaros saaros commented Aug 25, 2015

Plus refactoring and a few minor fixes.

Moved ClusterMonitor ands its dependencies as-is to a new file.
Moved WebServer and its dependencies as-is to a new file.
Refactored some code to make it easier to test it and simplified cases where
we use optional values from dicts, including removing code that could was
never executed.
timedelta.seconds only returns the seconds-part of the delta and doesn't
consider days (or microseconds).  Use the total_seconds() function instead
which takes all parts of the delta in account.  This fixes cases where we
had > 1 day replication lag and could have incorrectly returned low results.
Usually this was probably not an issue as replication lag rarely grows
beyond a day, and even if it does the window where previous day's time
matches current time is short.

NOTE: we can't use total_seconds() directly from timedelta objects as it's
not available on Python 2.6.  Implement a compat function in common.py.
@saaros saaros force-pushed the refactoring-and-fixes branch 2 times, most recently from 57628bc to 2c7b74d Compare August 25, 2015 13:37
melor added a commit that referenced this pull request Aug 25, 2015
timestamps: fix second calculation between datetime objects
@melor melor merged commit 0b7f4fb into Aiven-Open:master Aug 25, 2015
@saaros saaros deleted the refactoring-and-fixes branch August 25, 2015 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants