Policy bot stuck on `Commit hash does not have a pushed date` #598

joseparajelesGL · 2023-07-06T23:48:23Z

Hello, we are seeing errors on policy bot where it's stuck on trying to get the pushed date

Is there anything we can do to debug this issue?

Attached is the stack trace

failed to compute approval status: failed to list commits: Commit f3bed522f4 does not have a pushed date. This is usually caused by delays in the GitHub API
github.com/palantir/policy-bot/policy/approval.(*Rule).filteredCommits
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:345
github.com/palantir/policy-bot/policy/approval.(*Rule).IsApproved
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:204
github.com/palantir/policy-bot/policy/approval.(*Rule).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/approve.go:141
github.com/palantir/policy-bot/policy/approval.(*RuleRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:63
github.com/palantir/policy-bot/policy/approval.(*OrRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:86
github.com/palantir/policy-bot/policy/approval.(*AndRequirement).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:146
github.com/palantir/policy-bot/policy/approval.(*evaluator).Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/approval/evaluate.go:39
github.com/palantir/policy-bot/policy.evaluator.Evaluate
	/home/runner/work/policy-bot/policy-bot/policy/policy.go:80
github.com/palantir/policy-bot/server/handler.(*EvalContext).EvaluatePolicy
	/home/runner/work/policy-bot/policy-bot/server/handler/eval_context.go:125
github.com/palantir/policy-bot/server/handler.(*Details).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/server/handler/details.go:132
github.com/bluekeyes/hatpear.Try.func1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:84
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
github.com/palantir/policy-bot/server/handler.RequireLogin.func1.1
	/home/runner/work/policy-bot/policy-bot/server/handler/login.go:95
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74
goji%2eio.dispatch.ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/dispatch.go:17
github.com/bluekeyes/hatpear.Recover.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:113
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/bluekeyes/hatpear.Catch.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/bluekeyes/hatpear/hatpear.go:60
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/palantir/go-baseapp/baseapp.AccessHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/palantir/go-baseapp/baseapp/middleware.go:88
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/rs/zerolog/hlog.RequestIDHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/rs/zerolog/hlog/hlog.go:187
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/palantir/go-baseapp/baseapp.NewMetricsHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/palantir/go-baseapp/baseapp/middleware.go:55
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
github.com/rs/zerolog/hlog.NewHandler.func1.1
	/home/runner/work/policy-bot/policy-bot/vendor/github.com/rs/zerolog/hlog/hlog.go:29
net/http.HandlerFunc.ServeHTTP
	/opt/hostedtoolcache/go/1.20.4/x64/src/net/http/server.go:2122
goji%2eio.(*Mux).ServeHTTP
	/home/runner/work/policy-bot/policy-bot/vendor/goji.io/mux.go:74

The text was updated successfully, but these errors were encountered:

ab77 · 2023-07-07T00:09:38Z

According to this, a breaking change was made to Commit.pushedDate and pushedDate appears to be no longer supported.

esmelser-apex · 2023-07-07T00:11:14Z

We are also experiencing this issue and would love to see a fix. ➕ 1️⃣

johnwonkim · 2023-07-07T00:20:34Z

+1 as well.

zliu2-wish · 2023-07-07T02:19:50Z

We are hitting the issue as well. Don't have context into the repo but thanks to @ab77 did some digging in the code and found PushedDate being used:

policy-bot/pull/github.go

Line 925 in 51b4afb

PushedDate *time.Time

Looking at the graphql docs on Commit that field did get deprecated: https://docs.github.com/en/graphql/reference/objects#commit

I went through all the fields looking for possible replacements and found these fields:
authoredDate
committedDate

Could consider swapping out the pushDate for the above two fields but one possible issue I see:

if the pushDate was used to determine when to reset approvals, using the other two fields allows someone to author/commit early but hold the change locally
The user would be able to submit v1 of the changes (but already have v2 locally). Get the approval for v1 then push out v2. Since the v2 author/commit time was before the approval, it wouldn't reset the approvals

asvoboda · 2023-07-07T09:18:26Z

Thanks for the report. Its definitely a shame that the field has been removed from the graphql api:

Breaking A change will be made to Commit.pushedDate.
Description:
pushedDate will be removed.

Reason:
pushedDate is no longer supported.

You are correct @zliu2-wish that the PushedDate is used to determine approvals with the invalidate_on_push option, so this breaking change is particularly problematic.

asvoboda · 2023-07-07T09:48:35Z

I'm going to file a ticket with our enterprise reps to better understand what options we have on the GH side (#2241100).

I believe this should only affect policies where invalidate_on_push is used. I think the only short term option at the moment is to write a policy that has a fallback OR condition where a maintainer/admin has approved. I'll continue thinking about this.

See: palantir/policy-bot#598

asvoboda · 2023-07-07T11:54:09Z

I think we'll want to create an option to disable the push date loading for commits that can be toggled at the server level. Right now we attempt to load the pushed dates for commits even for rules that don't use the invalidate_on_push option, which is contributing to slowness in requests.

asvoboda · 2023-07-07T12:51:56Z

From support:

The Commit.pushedDate field was deprecated because the information calculated and presented is incorrect. The noticed was first announced in our changelog here and then deprecation happen 2023-07-01 as you already noted.

As I understand, there is currently no way to map a commit to a push, and establish that it's the first time the commit has been seen. There isn't a similar date object elsewhere in the API that can replace this.

We hope one day to offer some mapping from commit to push as it is on our radar but this isn't something that will be prioritized in the short/medium term.

DepickereSven · 2023-07-07T14:00:58Z

From support:

The Commit.pushedDate field was deprecated because the information calculated and presented is incorrect. The noticed was first announced in our changelog here and then deprecation happen 2023-07-01 as you already noted.

As I understand, there is currently no way to map a commit to a push, and establish that it's the first time the commit has been seen. There isn't a similar date object elsewhere in the API that can replace this.

We hope one day to offer some mapping from commit to push as it is on our radar but this isn't something that will be prioritized in the short/medium term.

So this would mean that for now there is no solution in the near future to be able to use invalidate_on_push?

asvoboda · 2023-07-07T14:06:07Z

Thats correct; it looks like on github.com support for invalidate on push has been entirely broken.

DepickereSven · 2023-07-07T14:20:14Z

Thats correct; it looks like on github.com support for invalidate on push has been entirely broken.

That's sad to hear because that was powerful in combination with the property ignore_commits_by.
So now when we switch back to letting GitHub decide to invalidate on push that property will no longer work either.

asvoboda · 2023-07-07T14:46:43Z

More information from support:

We discovered that the Commit GraphQL object returns a PushedDate field. This field is calculated in a problematic way. We do not have a way to tie a commit back to a specific push, so we cannot reliably calculate the first time we see a commit and which push it belongs to.

Instead, this field attempts to do it by finding the first Push in the pushes table across all repositories that has an after SHA that matches the commit in question. This doesn't work if the commit is somewhere in the middle of a group of commits in a push. It's possible that the commit's SHA isn't ever a push's after SHA. It's also possible that we do find a push with the matching after SHA, but it's not the first time we saw the commit.

All this to say, the field is very unreliable, and not accurate.

This probably explains why policy-bot needs a significant amount of code to handle pushedDate unreliability in https://github.com/palantir/policy-bot/blob/develop/pull/github.go#L856-L875

samandmoore · 2023-07-07T15:34:24Z

Wow. Well this is quite unfortunate. It's especially weird because GitHub seems to support similar behavior with their "dismiss stale reviews" branch protection rule setting. It clearly dismisses reviews older than the latest pushed changes on a PR.

lamebear · 2023-07-07T15:37:32Z

Could policy bot be updated so that if:

(github.PullRequestEvent).GetAction() == "synchronize", and
github.PullRequestEvent.PullRequest.Head.SHA has changed

it invalidates on push?

bluekeyes · 2023-07-07T16:19:06Z

Unfortunately, the way Policy Bot evaluation currently works requires being able to order commits and comments at any time, which means we can't easily switch to invalidating based on events without a larger refactor.

I do think that's how we'll have to fix this, but I'll have to think a bit about how to best do it. I think we'll have to store state somehow, but I'd really like to avoid introducing an external database. I think we can do something by looking at the times of previous statuses posted by Policy Bot, but I'm not clear on all the details yet.

asvoboda · 2023-07-07T16:32:13Z

This should now be available as docker.io/palantirtechnologies/policy-bot:snapshot if anyone is interested in trying it out. I think we should leave this issue open to continue to track further improvements and attempt to correctly invalidate commits pushed after approvals.

From github support, it looks like this behaviour won't land in GHES until 3.11, which is at least one more major version away (likely in 2024 based on the timing of previous releases).

agirlnamedsophia · 2023-07-07T17:11:09Z

@asvoboda this appears to resolve our issues! thank you.

joseparajelesGL · 2023-07-07T17:59:55Z

Can confirm as well, latest snapshot version works!
Thank you so much @asvoboda

zliu2-wish · 2023-07-07T18:42:37Z

Thanks @asvoboda!
Quick question - I have the below config option (following the sample setup) with the latest snapshot and am seeing this error:
failed to compute approval status: failed to list commits: Commit 0403ce40f4 does not have a pushed date. This is usually caused by delays in the GitHub API

I've removed all references to invalidate_on_push in the .policy.yml file. Am I missing something in the config?

options:
  # The path within repositories to find the policy.yml file
  policy_path: .policy.yml
  # The context for status checks created by the bot
  status_check_context: policy-bot
  # The name of the application as registered with GitHub
  app_name: policy-bot

  # As of 2023-07-01 the commit.pushedDate graphql field is removed from GitHub.
  # Setting this option effectively breaks all usage of the invalidate_on_push approval rule
  do_not_load_commit_pushed_date: true

asvoboda · 2023-07-07T18:50:52Z

@zliu2-wish double check you're using the latest version? https://hub.docker.com/layers/palantirtechnologies/policy-bot/snapshot/images/sha256-4ebb3c982f8601aceaf78886f956ca95e04c0cf37d5a1dd0ec0a346b46049fe9?context=explore

zliu2-wish · 2023-07-07T19:08:40Z

root@policy-bot-0ac779999e81f3372:~# docker container ls
CONTAINER ID        IMAGE                                      COMMAND                  CREATED             STATUS                 PORTS               NAMES
2847c13056ed        palantirtechnologies/policy-bot:snapshot   "bin/linux-amd64/pol…"   2 hours ago         Up 2 hours                                 docker_policy-bot

root@policy-bot-0ac779999e81f3372:/# docker image ls
REPOSITORY                        TAG                 IMAGE ID            CREATED             SIZE
palantirtechnologies/policy-bot   snapshot            4f2e6a7e0ca4        3 hours ago         36.6MB
palantirtechnologies/policy-bot   1.25.0              5d5e389db35d        14 months ago       16.9MB
root@policy-bot-0ac779999e81f3372:/# docker pull palantirtechnologies/policy-bot:snapshot
snapshot: Pulling from palantirtechnologies/policy-bot
Digest: sha256:4ebb3c982f8601aceaf78886f956ca95e04c0cf37d5a1dd0ec0a346b46049fe9
Status: Image is up to date for palantirtechnologies/policy-bot:snapshot

Will try to see if using the env variable option would help

sawyerward · 2023-07-07T20:13:23Z

@asvoboda It's working for us as well. Thank you.

zliu2-wish · 2023-07-07T20:18:07Z

@asvoboda

Update: adding the env variable (POLICYBOT_OPTIONS_DO_NOT_LOAD_COMMIT_PUSHED_DATE: true) makes it work for the repo that had invalidate_on_push fully removed from .policy.yml

However, for repos with invalidate_on_push still present, am getting this error now:

failed to filter candidates: no commit contained a push date

Going to go through the repos and remove the references one by one

Much appreciated for the fix

…nges Change-type: patch See: palantir/policy-bot#598 See: https://docs.github.com/en/graphql/overview/breaking-changes#changes-scheduled-for-2023-07-01

RyanDinglei · 2023-07-08T02:38:25Z

works for me. Many thanks.

gpadavala · 2023-07-10T13:34:29Z

i just tried the snapshot tag "palantirtechnologies/policy-bot:snapshot" with digest "sha256:5e14d46e0bf2ea178f11c6d0b9668131ac9644e88229b25ea68c13f7f087a68e"

I still have the same error

EDIT: seems will have to disable the invalidate_on_push, we have 100's of repositories, so not convenient to change them all though

waynechowhk01 · 2023-07-11T07:32:09Z

Thank you @asvoboda. The patch and ENV work with invalidate_on_push disabled.
To skip the action for those repositories, I have forked and make a simple workaround removing the r.Options.InvalidateOnPush function PR
The image is available at https://hub.docker.com/r/waynechowhk01/policy-bot/tags . Feel free to use it with
docker pull waynechowhk01/policy-bot:snapshot

This is to resolve an isue with policybot:latest using a field that was removed from Github's API. See palantir/policy-bot#598 for reference

See: palantir/policy-bot#598

rallydan · 2023-07-27T17:28:36Z

@bluekeyes, thanks for this fix (and the caching optimization in #612). Is there some testing or other development still pending, or would it be possible to get a release tag with these changes?

bluekeyes · 2023-07-27T17:37:38Z

I'd like to run one more test pass against our internal staging environment to verify everything before release. I'm planning to do that today, so hopefully I can tag a release by the end of the day (PDT).

bluekeyes · 2023-07-27T21:34:17Z

A permanent fix for this issue is now available in version 1.31.0. If you previously removed the invalidate_on_push option from your policy files, it should be safe to re-add them after upgrading to this version. While you can continue to the set the do_not_load_commit_pushed_date option, it's ignored by the server and has no effect.

If you encounter any issues with invalidate_on_push as a result of the new implementation, please file a new issue and we'll take a look.

Resolves: palantir/policy-bot#598 Change-type: patch

paulgessinger added a commit to acts-project/acts that referenced this issue Jul 7, 2023

ci: Disable validate on push for policybot

d6c23c6

See: palantir/policy-bot#598

asvoboda mentioned this issue Jul 7, 2023

Add a server option to avoid loading pushed dates #599

Merged

asvoboda closed this as completed in #599 Jul 7, 2023

asvoboda reopened this Jul 7, 2023

asvoboda added the bug Something isn't working label Jul 7, 2023

thgreasi mentioned this issue Jul 7, 2023

Dockerfile: Update policybot to latest snapshot to fix GH graphql changes product-os/policy-bot#3

Merged

bluekeyes mentioned this issue Jul 10, 2023

Implement invalidate_on_push without pushedDate field #602

Merged

ab77 mentioned this issue Jul 10, 2023

workaround for missing pushDate product-os/policies#40

Merged

beomki-yeo mentioned this issue Jul 20, 2023

Disable validate on push for policybot acts-project/traccc#432

Merged

paulgessinger added a commit to paulgessinger/acts that referenced this issue Jul 24, 2023

ci: Disable validate on push for policybot

8acfbf2

See: palantir/policy-bot#598

bluekeyes closed this as completed in #602 Jul 24, 2023

klutchell added a commit to product-os/policy-bot that referenced this issue Jul 31, 2023

Update to v1.31.0

ef19b26

Resolves: palantir/policy-bot#598 Change-type: patch

klutchell mentioned this issue Jul 31, 2023

Update to v1.31.0 product-os/policy-bot#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Policy bot stuck on `Commit hash does not have a pushed date` #598

Policy bot stuck on `Commit hash does not have a pushed date` #598

joseparajelesGL commented Jul 6, 2023

ab77 commented Jul 7, 2023

esmelser-apex commented Jul 7, 2023

johnwonkim commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023

asvoboda commented Jul 7, 2023 •

edited

Loading

asvoboda commented Jul 7, 2023 •

edited

Loading

asvoboda commented Jul 7, 2023

asvoboda commented Jul 7, 2023 •

edited

Loading

DepickereSven commented Jul 7, 2023

asvoboda commented Jul 7, 2023

DepickereSven commented Jul 7, 2023

asvoboda commented Jul 7, 2023

samandmoore commented Jul 7, 2023

lamebear commented Jul 7, 2023 •

edited

Loading

bluekeyes commented Jul 7, 2023

asvoboda commented Jul 7, 2023

agirlnamedsophia commented Jul 7, 2023

joseparajelesGL commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 •

edited

Loading

asvoboda commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 •

edited

Loading

sawyerward commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 •

edited

Loading

RyanDinglei commented Jul 8, 2023

gpadavala commented Jul 10, 2023 •

edited

Loading

waynechowhk01 commented Jul 11, 2023

rallydan commented Jul 27, 2023

bluekeyes commented Jul 27, 2023

bluekeyes commented Jul 27, 2023

Policy bot stuck on Commit hash does not have a pushed date #598

Policy bot stuck on Commit hash does not have a pushed date #598

Comments

joseparajelesGL commented Jul 6, 2023

ab77 commented Jul 7, 2023

esmelser-apex commented Jul 7, 2023

johnwonkim commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023

asvoboda commented Jul 7, 2023 • edited Loading

asvoboda commented Jul 7, 2023 • edited Loading

asvoboda commented Jul 7, 2023

asvoboda commented Jul 7, 2023 • edited Loading

DepickereSven commented Jul 7, 2023

asvoboda commented Jul 7, 2023

DepickereSven commented Jul 7, 2023

asvoboda commented Jul 7, 2023

samandmoore commented Jul 7, 2023

lamebear commented Jul 7, 2023 • edited Loading

bluekeyes commented Jul 7, 2023

asvoboda commented Jul 7, 2023

agirlnamedsophia commented Jul 7, 2023

joseparajelesGL commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 • edited Loading

asvoboda commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 • edited Loading

sawyerward commented Jul 7, 2023

zliu2-wish commented Jul 7, 2023 • edited Loading

RyanDinglei commented Jul 8, 2023

gpadavala commented Jul 10, 2023 • edited Loading

waynechowhk01 commented Jul 11, 2023

rallydan commented Jul 27, 2023

bluekeyes commented Jul 27, 2023

bluekeyes commented Jul 27, 2023

Policy bot stuck on `Commit hash does not have a pushed date` #598

Policy bot stuck on `Commit hash does not have a pushed date` #598

asvoboda commented Jul 7, 2023 •

edited

Loading

asvoboda commented Jul 7, 2023 •

edited

Loading

asvoboda commented Jul 7, 2023 •

edited

Loading

lamebear commented Jul 7, 2023 •

edited

Loading

zliu2-wish commented Jul 7, 2023 •

edited

Loading

zliu2-wish commented Jul 7, 2023 •

edited

Loading

zliu2-wish commented Jul 7, 2023 •

edited

Loading

gpadavala commented Jul 10, 2023 •

edited

Loading