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

Do you support ignoring in progress builds? #781

Open
alita-moore opened this issue Jan 18, 2022 · 16 comments
Open

Do you support ignoring in progress builds? #781

alita-moore opened this issue Jan 18, 2022 · 16 comments

Comments

@alita-moore
Copy link

I'm having an issue where I have two PRs

PR A | required check 1 [passed] | required check 2 [passed] | kodiak [second in queue for merge]
PR B | required check 1 [passed] | required check 2 [in progress] | kodiak [waiting for check 2 to pass]

Check 2 in PR B will never complete (due to some issue out of my control) which will then prevent all other ready to merge PRs from merging.

It would be better if Kodiak simply ignored PR B and then merged PR A and only actually added a given PR to queue if and only if all required checks have passed and no other criteria but passed.

Is this functionality already supported?

@chdsbd
Copy link
Owner

chdsbd commented Jan 18, 2022

Does this merge.dont_wait_on_status_checks setting work for your use case?

https://kodiakhq.com/docs/config-reference#mergedont_wait_on_status_checks

This way, if you have a status check that never finishes, like WIP, then you can specify that check in this config and Kodiak won't wait for the check to finish.

@alita-moore
Copy link
Author

alita-moore commented Jan 18, 2022

by "not wait" do you mean it will merge it even if it's in progress?

but I'll give it a go!

@chdsbd
Copy link
Owner

chdsbd commented Jan 19, 2022

@alita-moore Kodiak always obeys GitHub Branch Protection, so if a status check is required, Kodiak won’t be able to merge the PR until it passes

That setting I linked will cause Kodiak to skip merging a PR while they status check is in progress

@alita-moore
Copy link
Author

awesome, thank you for clarifying! It may be helpful to include that clarifying point in the docs btw. I wasn't sure if it meant it would ignore that required check.

@alita-moore
Copy link
Author

@chdsbd do you know why this is not merging? I think I may have misunderstood how the setting works. ethereum/EIPs#4700

@alita-moore alita-moore reopened this Jan 19, 2022
@alita-moore
Copy link
Author

okay I have confirmed that this did not work because of kodiak timing out

@chdsbd
Copy link
Owner

chdsbd commented Jan 19, 2022

Looking in the logs, Kodiak set the not waiting for dont_wait_on_status_checks status message at 19:38:25 ET, but the EIP Auto-Merge Bot was marked as successful at 19:37 ET. So I think Kodiak saw the old status check, did its thing and then set a new status check.

GitHub's API is also eventually consistent, so that can cause issues like this too.

As a workaround, you can edit the PR title, body, labels to "kick" Kodiak to reevaluate the PR: https://kodiakhq.com/docs/troubleshooting#workarounds

@alita-moore
Copy link
Author

alita-moore commented Jan 19, 2022

Unfortunately that workaround won't work for the use case. Is there any way to just "kick" it awake every so often automatically in case something like this happens?

@alita-moore
Copy link
Author

also the other required check (Travis CI) didn't pass until 42 so it would make sense that it was marked that status at that time

@alita-moore
Copy link
Author

huh, okay I think I understand the logic of this feature now. I understand it that it waits for a certain amount of time and then if it doesn't pass it fails it. Do you only get one thread per repo? If that's the case then can we either
a) increase the wait time
b) periodically sync kodiak

@chdsbd
Copy link
Owner

chdsbd commented Jan 19, 2022

This isn't the normal Kodiak behavior. You shouldn't normally have this issue.

Typically you will never have to futz with Kodiak, but when the GitHub Api flakes out, you may need to kick Kodiak.

If you have a merge queue enabled (the default), then Kodiak will use a single Python asyncio task per repository branch target. This is pretty close to an OS thread.

@alita-moore
Copy link
Author

Is there a programmatic way to avoid this problem?

@chdsbd
Copy link
Owner

chdsbd commented Jan 19, 2022

@alita-moore Yeah, I'm sure we could build a better solution. It's not something that we've invested in because it hasn't been that big of an issue.

Currently Kodiak only evaluates a PR whenever there is a webhook. There's some retries, but if GitHub flakes out then Kodiak won't evaluate a PR until there's another update

@sbdchd
Copy link
Collaborator

sbdchd commented Jan 19, 2022

Another thing Kodiak has to balance is rate limits. The GitHub API has rather strict rate limits so Kodiak has to keep track of API requests to ensure it doesn't go over.

I think a background, anti-entropy style check could work, but there is a cost

@alita-moore
Copy link
Author

makes sense. I've worked a lot with the rest api, so I understand all about it. The challenge with my use case is that it really needs to auto-merge as close to 100% reliably as possible. Which is why I came to kodiak (and many other externally maintained solutions). It's unfortunate that github does not support this use case correctly.

Assuming that this becomes a big enough problem, I would be happy to help develop that other solution.

@alita-moore
Copy link
Author

@chdsbd @sbdchd it looks like this is a consistent problem. You can see more examples with pull requests that have not auto merged. ethereum/EIPs#4704

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants