Skip to content

fix: improving geolite workflow#37706

Merged
feanil merged 3 commits intoopenedx:masterfrom
ktyagiapphelix2u:ktyagi/geolite
Dec 12, 2025
Merged

fix: improving geolite workflow#37706
feanil merged 3 commits intoopenedx:masterfrom
ktyagiapphelix2u:ktyagi/geolite

Conversation

@ktyagiapphelix2u
Copy link
Contributor

Description

We changed the branch name logic because the workflow regenerated the same branch name on reruns (since it was based on github.sha), causing GitHub to reject the push with “fetch first” because the branch already existed on the remote.

@ktyagiapphelix2u ktyagiapphelix2u marked this pull request as ready for review December 1, 2025 05:34
@ktyagiapphelix2u
Copy link
Contributor Author

@feanil

When using github.sha, the workflow passes the first time, but fails when rerun because it throws a “fetch first” error. This happens because the PR created by the previous successful workflow run is still open or unmerged, so the branch already exists.
I made an improvement to it, and it seems logical to me, could you take a look?

Use github.sha never changes when you rerun the workflow on the same commit, the workflow kept generating the same branch name, the push failed with a “fetch first” error.

@feanil
Copy link
Contributor

feanil commented Dec 2, 2025

@ktyagiapphelix2u can you give me some more info? Doesn't that just mean that a PR already exists that should get merged instead of making a new one? Or is the goal here to replace the old PR with a new one because the old one is out of date?

@ktyagiapphelix2u
Copy link
Contributor Author

@feanil
edx#40 This is the reference PR for the same work we were doing to fix and update the GeoLite workflow. The issue we ran into was that when using github.sha, the workflow succeeds the first time but fails when rerun, throwing a “fetch first” error. This happens because the PR created during the previous successful run is still open or unmerged, so the branch already exists.
I’ve made an improvement that seems logically correct to me

Since github.sha doesn’t change when rerunning the workflow on the same commit, the workflow kept generating the same branch name, causing the push to fail with a “fetch first” error.

https://2u-internal.atlassian.net/browse/BOMS-267
This was the related ticket for the same

@feanil
Copy link
Contributor

feanil commented Dec 9, 2025

So is the issue you're trying to fix, that a PR is created with a version of the geolite database that's out of date? And you want to create a new PR manually but there have been no updates to the platform in the intervening time and the branch name you're trying to use already exists? The change seems harmless enough but I'm trying to understand the motivation behind the issue because I haven't run into why you need to keep re-running the workflow manually if a PR already exists. Why not merge the PR and then re-run to get further updates?

@ktyagiapphelix2u
Copy link
Contributor Author

@feanil Since this is an enhancement, I was considering it for future improvement. As you mentioned, it's harmless, so we can proceed with it. Otherwise, it's fine to leave things as they are and close the PR. Thanks.

@feanil feanil merged commit e96e6ec into openedx:master Dec 12, 2025
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants