From 7f204b8bc9dc8db47c06766a05bc71472e7815bb Mon Sep 17 00:00:00 2001 From: Rodos Date: Wed, 4 Nov 2026 09:34:38 +1100 Subject: [PATCH] Fix --incremental-by-files skipping updated items east of UTC The modification time of a saved issue, pull request or discussion was formatted in local time but labelled UTC, then compared with GitHub's UTC updated_at. East of UTC, items updated within the offset after they were saved were skipped and their changes missed; west of UTC, unchanged items were fetched again. The checkpoint time used when a repository has no updated_at or pushed_at had the same local-time mistake. --- github_backup/github_backup.py | 16 ++-- tests/test_incremental_by_files.py | 130 +++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 tests/test_incremental_by_files.py diff --git a/github_backup/github_backup.py b/github_backup/github_backup.py index 9fbfacf..d72317f 100644 --- a/github_backup/github_backup.py +++ b/github_backup/github_backup.py @@ -20,7 +20,7 @@ import time import traceback from collections.abc import Generator -from datetime import datetime +from datetime import datetime, timezone from http.client import IncompleteRead from urllib.error import HTTPError, URLError from urllib.parse import urlencode, urlparse @@ -2004,7 +2004,7 @@ def get_repository_checkpoint_time(repository): if timestamps: return max(timestamps) - return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.localtime()) + return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) def resource_backup_exists(resource_cwd): @@ -2525,7 +2525,9 @@ def backup_discussions(args, repo_cwd, repository): if args.incremental_by_files and os.path.isfile(discussion_file): modified = os.path.getmtime(discussion_file) - modified = datetime.fromtimestamp(modified).strftime("%Y-%m-%dT%H:%M:%SZ") + modified = datetime.fromtimestamp(modified, timezone.utc).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ) if modified > summary["updatedAt"]: logger.info( "Skipping discussion {0} because it wasn't modified since last backup".format( @@ -2805,7 +2807,9 @@ def backup_issues(args, repo_cwd, repository, repos_template): and number not in stale_timelines ): modified = os.path.getmtime(issue_file) - modified = datetime.fromtimestamp(modified).strftime("%Y-%m-%dT%H:%M:%SZ") + modified = datetime.fromtimestamp(modified, timezone.utc).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ) if modified > issue["updated_at"]: logger.info( "Skipping issue {0} because it wasn't modified since last backup".format( @@ -3006,7 +3010,9 @@ def pull_is_due_for_repository_checkpoint(pull): if args.incremental_by_files and os.path.isfile(pull_file): modified = os.path.getmtime(pull_file) - modified = datetime.fromtimestamp(modified).strftime("%Y-%m-%dT%H:%M:%SZ") + modified = datetime.fromtimestamp(modified, timezone.utc).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ) if modified > pull["updated_at"] and not needs_review_backfill: logger.info( "Skipping pull request {0} because it wasn't modified since last backup".format( diff --git a/tests/test_incremental_by_files.py b/tests/test_incremental_by_files.py new file mode 100644 index 0000000..ff3c5b4 --- /dev/null +++ b/tests/test_incremental_by_files.py @@ -0,0 +1,130 @@ +"""Tests for --incremental-by-files timestamp comparison. + +A saved file is skipped when its modification time is newer than the item's +``updated_at``. GitHub's timestamps are UTC, so the file's modification time +must be compared in UTC too, whatever the local time zone is. +""" + +import calendar +import json +import os +import time + +import pytest + +from github_backup import github_backup + +UPDATED_AT = "2026-07-01T12:00:00Z" + + +@pytest.fixture +def east_of_utc(monkeypatch): + """Run the test with the local time zone at UTC+10 (no daylight saving).""" + monkeypatch.setenv("TZ", "Etc/GMT-10") + time.tzset() + yield + monkeypatch.undo() + time.tzset() + + +def _stored(path, number, written_at): + """Save an item file whose modification time is ``written_at`` (UTC).""" + path.mkdir(parents=True, exist_ok=True) + item_file = path / "{0}.json".format(number) + item_file.write_text(json.dumps({"number": number, "title": "stored"})) + mtime = calendar.timegm(time.strptime(written_at, "%Y-%m-%dT%H:%M:%SZ")) + os.utime(item_file, (mtime, mtime)) + return item_file + + +def _fake_retrieve(items): + def _retrieve(args, template, query_args=None, **kwargs): + return items if template.endswith(("/issues", "/pulls")) else [] + + return _retrieve + + +@pytest.mark.parametrize( + "written_at, refetched", + [ + ("2026-07-01T10:00:00Z", True), # updated 2 hours after it was saved + ("2026-07-01T14:00:00Z", False), # saved 2 hours after the update + ], +) +def test_issue_skip_compares_in_utc( + create_args, tmp_path, monkeypatch, east_of_utc, written_at, refetched +): + issue_file = _stored(tmp_path / "issues", 1, written_at) + issue = {"number": 1, "title": "updated", "updated_at": UPDATED_AT} + monkeypatch.setattr(github_backup, "retrieve_data", _fake_retrieve([issue])) + args = create_args(include_issues=True, incremental_by_files=True, since=None) + + github_backup.backup_issues( + args, str(tmp_path), {"full_name": "owner/repo"}, "https://api.github.com/repos" + ) + + title = json.loads(issue_file.read_text())["title"] + assert title == ("updated" if refetched else "stored") + + +@pytest.mark.parametrize( + "written_at, refetched", + [ + ("2026-07-01T10:00:00Z", True), + ("2026-07-01T14:00:00Z", False), + ], +) +def test_pull_skip_compares_in_utc( + create_args, tmp_path, monkeypatch, east_of_utc, written_at, refetched +): + pull_file = _stored(tmp_path / "pulls", 1, written_at) + pull = {"number": 1, "title": "updated", "updated_at": UPDATED_AT} + monkeypatch.setattr(github_backup, "retrieve_data", _fake_retrieve([pull])) + args = create_args(include_pulls=True, incremental_by_files=True, since=None) + + github_backup.backup_pulls( + args, str(tmp_path), {"full_name": "owner/repo"}, "https://api.github.com/repos" + ) + + title = json.loads(pull_file.read_text())["title"] + assert title == ("updated" if refetched else "stored") + + +@pytest.mark.parametrize( + "written_at, refetched", + [ + ("2026-07-01T10:00:00Z", True), + ("2026-07-01T14:00:00Z", False), + ], +) +def test_discussion_skip_compares_in_utc( + create_args, tmp_path, monkeypatch, east_of_utc, written_at, refetched +): + discussion_file = _stored(tmp_path / "discussions", 1, written_at) + summary = {"number": 1, "updatedAt": UPDATED_AT} + monkeypatch.setattr( + github_backup, + "retrieve_discussion_summaries", + lambda args, repository, since=None: ([summary], UPDATED_AT, True, 1), + ) + monkeypatch.setattr( + github_backup, + "retrieve_discussion", + lambda args, repository, number: {"number": number, "title": "updated"}, + ) + args = create_args( + include_discussions=True, incremental_by_files=True, token_classic="ghp_test" + ) + + github_backup.backup_discussions(args, str(tmp_path), {"full_name": "owner/repo"}) + + title = json.loads(discussion_file.read_text())["title"] + assert title == ("updated" if refetched else "stored") + + +def test_checkpoint_fallback_time_is_utc(east_of_utc): + before = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + checkpoint = github_backup.get_repository_checkpoint_time({}) + after = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + + assert before <= checkpoint <= after