Pull request not created when using `wlc`

rix0rrr commented 2 months ago

Describe the issue

In order to prevent merge conflicts, we use automation workflows that use wlc to get Weblate to create PRs. Our workflow looks like this:

Use wlc repo to check if there are changes, stop if there aren't any
wlc lock all components
wlc pull
wlc commit
wlc push
wlc reset to force the repository back to a clean state, because the PR will be squash-merged and otherwise we would get merge conflicts on the next pull

When the PR is merged, a workflow runs automatically in the on_close event:

wlc unlock all components again

We notice that occasionally, even though the repository shows needs_commit: True, after running:

wlc commit
wlc push

No PR is created after all. Since no PR is created, our unlock workflow has no chance to run and the Weblate translations remain locked forever.

In the most recent case it was even worse: no PR was created, and as I mentioned wlc reset was run so all new translations were lost 😞 I figured this was safe because after running commit and push, all changes would end up in the PR, but it turns out that sometimes no PR is created at all.

A timestamp when this happened is 2024-09-01 06:15:32 UTC, on the Hedy project: changes were pending, but running commit and push didn't create a PR.

This is not consistent behavior. If I'd have to guess, I would say it happens in ~10% of cases... but with changes every day it happens often enough that it's quite disruptive for us.

What is the reason this happens? How can we write a script using wlc that will reliably create a PR if there are changes to commit?

I already tried

[X] I've read and searched the documentation.
[X] I've searched for similar filed issues in this repository.

Steps to reproduce the behavior

Write a script using WLC that looks like this:

wlc pull
wlc commit
wlc push

Make changes to a Weblate translation, and run the script above.

In about ~10% of cases, no PR is created.

Expected behavior

I would expect a PR to be created in every case.

Screenshots

No response

Exception traceback

No response

How do you run Weblate?

weblate.org service

Weblate versions

No response

Weblate deploy checks

No response

Additional context

No response

nijel commented 1 month ago

commit triggers background creation of the pull request (if push on commit is turned on), so push should not be needed. But there is no guarantee that it is started before reset. IMHO the reset should be executed upon pull request merge, not when creating it (Weblate is locked meanwhile anyway).

Do you have a log for the failed run? There might be something useful in wlc output there.

Moreover, you might want to use squashing at Weblate side to avoid reset completely (but that might need other workflow adjustments).

rix0rrr commented 1 month ago

"push on commit" is not turned on, because sometimes Weblate will commit of its own accord (and hence push), and I only want the PR created at defined times of the day.

Is the handling of wlc commit also asynchronous?

Do you have a log for the failed run? There might be something useful in wlc output there.

I did not run it with --debug, so I don't have a log on my end, just a timestamp. I was hoping you would have some server-side logs to look at.

Moreover, you might want to use squashing at Weblate side to avoid reset completely (but that might need other workflow adjustments).

Even if Weblate squashed on its end, it's unlikely it would come up with the exact same commit as GitHub would produce (same author, same commit message, etc). Yes we could regular-merge the pre-squashed commit, but it would need special scripting to handle Weblate PRs differently from other PRs (which are always squashed), and in any case we might need to do some final tweaks to Weblate PRs because we might need to adjust translations that have broken source code in them.

So there is really no way of avoiding the reset, I don't think.

IMHO the reset should be executed upon pull request merge

That's an idea! I suppose you're right that it shouldn't matter from the Weblate PoV. But I wonder if we'd run into an ordering problem there: the reset needs to be run before the PR is merged, because merging the PR updates the main branch, which Weblate would then pull and it would discover merge conflicts.

So we'd need the following steps in this order:

Weblate is locked
PR is created
....
PR is approved
Weblate is reset
PR is merged
(Weblate auto-pulls)
Weblate is unlocked

I'm not sure I can confidently script those events in GitHub.

nijel commented 1 month ago

If you use rebase in git, it will look at the content of the commit and will discards commits with the same content. So it doesn't really matter how the commit messages or authors look like.

Reset after merging the pull request is safe, even if there would be a merge conflict, it would be then reset to match upstream.

rix0rrr commented 1 month ago

Reset after merging the pull request is safe, even if there would be a merge conflict, it would be then reset to match upstream

Yes, but would the order not go like this:

PR is merged, leading to conflicts
Weblate will detect conflicts during auto-pull, and send an email saying "there are merge conflicts"?
Reset is run to discard Weblate state

?

If so, we'd have an email every day warning us about a non-problem, hiding the cases when there's an actual problem.

rix0rrr commented 1 month ago

If you use rebase in git, it will look at the content of the commit and will discards commits with the same content

I know. That check will fail if we happened to mutate the PR while merging, which can happen because the translations contain source code which might contain problems that need to be fixed.

It's a best-effort check that can fail, which is why I don't want to rely on it.

rix0rrr commented 1 month ago

Can you please tell me whether the handling of wlc commit is synchronous or asynchronous?

Thanks!

rix0rrr commented 1 month ago

One more question. If I do the following:

wlc commit
sleep 30
wlc push
sleep 60
wlc repo

I see the pull request being created.

Is it expected that I still see needs_push: True in response to the wlc repo command?

nijel commented 1 month ago

Right now wlc commit is synchronous. The possible triggered push is not.

I'm not sure about needs_push semantic right now. It might be also that the changes needs to be merged upstream. The flag is there from times when pushing was to the same branch and repo and its semantic was clearer back then.

rix0rrr commented 1 month ago

Got you, thanks.

I've changed to using the GitHub API to see if a PR was created.

I also noticed that there can be quite a delay between an upstream repository change and Weblate pulling the changes.

We had an interval of ~45 minutes the other day. Of course, a translator had translated in the interval between us merging the PR and unlocking Weblate, and Weblate actually performing the pull, so we had another merge conflict.

Do you have statistics on typical values for the notification-pull delays?

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because there wasn’t any recent activity.

It will be closed soon if no further action occurs.

Thank you for your contributions!

nijel commented 1 month ago

It's all asynchronous, so there can be delay on GitHub sending out the webhook and there can be delay in Weblate processing. You can see in Weblate history when the webhook was received, the processing is queued immediately, but it might take some time depending on server load.

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale because there wasn’t any recent activity.

It will be closed soon if no further action occurs.

Thank you for your contributions!

WeblateOrg / weblate