implement timestamp correction utilities and tests for git commit data#346
Merged
MoralCode merged 5 commits intoMay 27, 2026
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bandit found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Author
|
Thorough testing is needed given that this has been rebased since it was initially written/tested |
MoralCode
added
the
waiting
Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
…ction tests Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Addresses PR #3518 review feedback from Ulincsys: the db module must not import from the tasks layer, as it's intended to be pip-installable as a standalone package. Moved correction.py to augur/application/db/timestamp_utils.py and relocated its tests accordingly. Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
MoralCode
force-pushed
the
shlokgilda/fix/issue-3472-validate-committer-timestamps
branch
from
0537c1b to
fd57eb8
Compare
Contributor
Author
|
replicated the issue with the frr repo during analyze commits in parallel. now running with ~this branch (i introduced a bug). will consider it tested when i see the FRR repo hit full collection (or at least have analyze_commits_in_parallel not failing for it in flower AKA facade task success) |
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
MoralCode
force-pushed
the
shlokgilda/fix/issue-3472-validate-committer-timestamps
branch
from
fd57eb8 to
05f5eb6
Compare
MoralCode
removed
the
waiting
Contributor
Author
|
I observed analyze_commits_in_parallel task complete for this repo in flower. Test passed. |
MoralCode
deleted the
shlokgilda/fix/issue-3472-validate-committer-timestamps
branch
github-project-automation
Bot
moved this from In Progress
to Done
in CollectOSS Feature Roadmap
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Fixes #157. originally filed by @shlokgilda in augurlabs/augur#3518
The Bug:
facade_bulk_insert_commits()only validatedcmt_author_timestampwhen handling DB insertion failures. If the author timestamp was valid but the committer timestamp had an invalid timezone (e.g.,-13068837from git corruption), the insert would fail permanently.The Fix:
augur/tasks/git/correction.pywith timezone validation functions (simple string operations)lib.pyto validate both author and committer timestamps when the binary search isolates a problematic commitChanges:
augur/tasks/git/correction.py- Timestamp validation utilitiesaugur/application/db/lib.py- Now validates both timestamps on failuretests/test_tasks/test_git/test_correction.py- 12 unit tests (all passing)Notes for Reviewers
POSTGRES_VALID_TIMEZONESincludes all real-world timezone offsets (-12:00 to +14:00). Some of the timezone offsets were missing. Reference: https://docs.oracle.com/cd/E19563-01/819-4437/anovd/index.htmlSigned commits
AI Disclosure: I used Claude Code to assist with writing docstrings, code review, writing test cases, and drafting this PR description. All test cases were manually verified and are passing locally.