-
Notifications
You must be signed in to change notification settings - Fork 204
Fix PR push times #420
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
Fix PR push times #420
Changes from 8 commits
dff9d45
8af4b6c
9e692ac
eb9c97b
86ee2db
59eed0a
1e99e75
95dfe81
de493ed
5b21b7b
3e2902e
2f1f522
3483eb9
b057eb8
ecf0005
b345426
59c423a
dd1a6c8
d641d0e
b2d880d
12a9833
5b75484
7c65607
a270653
048a5b8
13ef07c
6d8579f
33a0a5d
999ae57
f24dbae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,14 +122,48 @@ def close_pr(api, urn, pr): | |
| return api("patch", path, json=data) | ||
|
|
||
|
|
||
| def get_pr_last_updated(pr_data): | ||
| def get_push_events(api, pr_owner, pr_repo): | ||
| """ | ||
| a helper for getting the github events on a given repo... useful for | ||
| finding out the last push on a repo. | ||
| """ | ||
| # TODO: this only gets us the latest 90 events, should we do more? | ||
| # i.e. can someone do 90 events on a repo in 30 seconds? | ||
| events = [] | ||
| for i in range(1, 4): | ||
| path = "/repos/{pr_owner}/{pr_repo}/events?page={page}".format( | ||
| pr_owner=pr_owner, pr_repo=pr_repo, page=i) | ||
| events += api("get", path, json={}) | ||
|
|
||
| return events | ||
|
|
||
|
|
||
| def get_pr_last_updated(api, pr_data): | ||
| """ a helper for finding the utc datetime of the last pr branch | ||
| modifications """ | ||
| repo = pr_data["head"]["repo"] | ||
| if repo: | ||
| return arrow.get(repo["pushed_at"]) | ||
| else: | ||
| return None | ||
|
|
||
| pr_ref = pr_data["head"]["ref"] | ||
| pr_repo = pr_data["head"]["repo"]["name"] | ||
| pr_owner = pr_data["user"]["login"] | ||
|
|
||
| events = get_push_events(api, pr_owner, pr_repo) | ||
| events = list(filter(lambda e: e["type"] == "PushEvent", events)) | ||
|
|
||
| # Gives the full ref name "ref/heads/my_branch_name", but we just | ||
| # want my_branch_name, so isolate it... | ||
| events = list(filter(lambda e: e["payload"]["ref"].split("/")[-1] == pr_ref, events)) | ||
|
||
|
|
||
| if len(events) == 0: | ||
| # if we can't get good data, fall back to repo push time | ||
| repo = pr_data["head"]["repo"] | ||
| if repo: | ||
| return arrow.get(repo["pushed_at"]) | ||
|
||
| else: | ||
| return None | ||
|
|
||
| last_updated = max(sorted(map(lambda e: e["created_at"], events))) | ||
|
|
||
| return arrow.get(last_updated) | ||
|
|
||
|
|
||
| def get_pr_comments(api, urn, pr_num): | ||
|
|
@@ -176,7 +210,7 @@ def get_ready_prs(api, urn, window): | |
| pr_num = pr["number"] | ||
|
|
||
| now = arrow.utcnow() | ||
| updated = get_pr_last_updated(pr) | ||
| updated = get_pr_last_updated(api, pr) | ||
| if updated is None: | ||
| comments.leave_deleted_comment(api, urn, pr["number"]) | ||
| close_pr(api, urn, pr) | ||
|
|
@@ -211,17 +245,17 @@ def get_ready_prs(api, urn, window): | |
| close_pr(api, urn, pr) | ||
|
|
||
|
|
||
| def voting_window_remaining_seconds(pr, window): | ||
| def voting_window_remaining_seconds(api, pr, window): | ||
| now = arrow.utcnow() | ||
| updated = get_pr_last_updated(pr) | ||
| updated = get_pr_last_updated(api, pr) | ||
| if updated is None: | ||
| return math.inf | ||
| delta = (now - updated).total_seconds() | ||
| return window - delta | ||
|
|
||
|
|
||
| def is_pr_in_voting_window(pr, window): | ||
| return voting_window_remaining_seconds(pr, window) <= 0 | ||
| def is_pr_in_voting_window(api, pr, window): | ||
| return voting_window_remaining_seconds(api, pr, window) <= 0 | ||
|
|
||
|
|
||
| def get_pr_reviews(api, urn, pr_num): | ||
|
|
@@ -272,7 +306,7 @@ def post_accepted_status(api, urn, pr, voting_window, votes, total, threshold, | |
| meritocracy_satisfied): | ||
| sha = pr["head"]["sha"] | ||
|
|
||
| remaining_seconds = voting_window_remaining_seconds(pr, voting_window) | ||
| remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window) | ||
| remaining_human = misc.seconds_to_human(remaining_seconds) | ||
| votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied) | ||
|
|
||
|
|
@@ -284,7 +318,7 @@ def post_rejected_status(api, urn, pr, voting_window, votes, total, threshold, | |
| meritocracy_satisfied): | ||
| sha = pr["head"]["sha"] | ||
|
|
||
| remaining_seconds = voting_window_remaining_seconds(pr, voting_window) | ||
| remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window) | ||
| remaining_human = misc.seconds_to_human(remaining_seconds) | ||
| votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied) | ||
|
|
||
|
|
@@ -296,7 +330,7 @@ def post_pending_status(api, urn, pr, voting_window, votes, total, threshold, | |
| meritocracy_satisfied): | ||
| sha = pr["head"]["sha"] | ||
|
|
||
| remaining_seconds = voting_window_remaining_seconds(pr, voting_window) | ||
| remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window) | ||
| remaining_human = misc.seconds_to_human(remaining_seconds) | ||
| votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,13 @@ def create_mock_pr(number, title, pushed_at): | |
| "statuses_url": "statuses_url/{}".format(number), | ||
| "head": { | ||
| "repo": { | ||
| "pushed_at": pushed_at | ||
| } | ||
| "pushed_at": pushed_at, | ||
| "name": "test_repo", | ||
| }, | ||
| "ref" : "test_ref" | ||
|
||
| }, | ||
| "user": { | ||
| "login": "test_user", | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to turn it into a list here? I think
filtercan take an iterator.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, I was printing the value so I need it as a list. Will fix