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

[Feature Request] Automatic Creation of Pull Requests for the prod Branch #3280

Open
prudhvigodithi opened this issue Sep 9, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@prudhvigodithi
Copy link
Contributor

prudhvigodithi commented Sep 9, 2024

Is your feature request related to a problem? Please describe

Today, the pull requests are created in the main branch and then merged into the prod branch. Sometimes, the merge to prod is not immediate, causing commits to pile up and leading to bulk merges into the prod branch.

Describe the solution you'd like to see

Have an automation with label backport prod (added to the PR) and onboard the backport.yml so that when the label is added the PR is automatically raised to prod.

Describe alternatives you've considered

No response

Additional context

I'm open for discussion if we need to call it as backport prod or frontport prod label. If we are going with frontport we need to explore the backport.yml to ensure it responds to frontport prod label

@prudhvigodithi prudhvigodithi added enhancement New feature or request untriaged labels Sep 9, 2024
@prudhvigodithi
Copy link
Contributor Author

@peterzhuamazon
Copy link
Member

It does make sense to me that each merge in main branch should result in a individual PR to prod branch, so that maintainer and cherry-pick which one to merge without the issue of one needs to go in earlier but mixed with other commits.

Thanks.

@krisfreedain
Copy link
Member

While I can see why you would want this as a feature request, this would likely result in even more of a backlog on busy days, and, potential out-of-sinc issues with back end processes. It would likely also slow things down on busy days. It takes approximately 13 minutes from merge to show on the staging site, then a maintainer would do a push to prod PR, merge that, and it take another 13 minutes to get to the prod website. As maintainers, there are times when we push things all the way to prod quickly, or, there are times when we will control what is going out - this could include release days, an important blog announcement, site changes, etc. Having a prod merge for every merge in main would add levels of confusion non-maintainers likely don't quite see in the normal back-end processes. Also, when there are things on staging that haven't been pushed to prod, the latest push to prod PR picks all of it up. Having all these auto-created push to prod PRs created will result in a mass of duplicated PRs in the repo, adding another layer of complexity and maintenance for maintainers to manage. No while this sounds like an OK idea, it really isn't something we would want to do.

@nateynateynate
Copy link
Member

I suspect this is more of a problem of maintainer availability more than it is the CI/CD. If we have time sensitive PR's we should build a mechanism to get them merged in a timely manner rather than remove the staging component of our publishing workflow. There are many times that we want things on a staging server prior to merging to production for the purposes of QA.

@nateynateynate
Copy link
Member

nateynateynate commented Sep 10, 2024

I think I'm misreading this. Are you suggesting that a PR to prod be created for each individual PR that merges to main (that has the proposed label) so that commits to prod are atomic (with only the changes of that individual commit to main ) and not just make a pr to merge everything (which is currently what we do now)?

I'm not entirely opposed to this however its implementation is confusing to me. Do we incur the risk of more conflicting merges? Merging them in a different order than they were originally merged to main ?

If we merge "x" Pr's to prod, will our build pipeline trigger "x" times in a row?

Do we retain the option of using our previous workflow if we want to just merge main into prod as a whole?

@prudhvigodithi
Copy link
Contributor Author

Thanks @nateynateynate @krisfreedain and @peterzhuamazon

Are you suggesting that a PR to prod be created for each individual PR that merges to main (that has the proposed label) so that commits to prod are atomic (with only the changes of that individual commit to main ) and not just make a pr to merge everything (which is currently what we do now)?

Thats true Nate, here is an example PR from another repo opensearch-project/job-scheduler#674 (using the same workflow), where one backport PR just targets the individual PR that was merged to main.

Do we incur the risk of more conflicting merges? Merging them in a different order than they were originally merged to main ?

Yes sometimes the backport fails (Example opensearch-project/job-scheduler#534 (comment)) and or if the backport PR is open for a while it might lead to conflicts, then a manual intervention is required on the PR. Following are some actions that can be taken.

- Close the automation PR and create one backport PR manually.
- Close the automation PR and now on the original PR remove and re-add the backport label again, this will re-create a new backport PR again (a hacky way).

If we merge "x" Pr's to prod, will our build pipeline trigger "x" times in a row?

For this adding @peterzhuamazon @zelinh to please provide more inputs.

Do we retain the option of using our previous workflow if we want to just merge main into prod as a whole?

Oh we used to have this? like merging a set of PR's to the prod branch rather than targeting individual PR's?

@prudhvigodithi
Copy link
Contributor Author

While I can see why you would want this as a feature request, this would likely result in even more of a backlog on busy days, and, potential out-of-sinc issues with back end processes. It would likely also slow things down on busy days. It takes approximately 13 minutes from merge to show on the staging site, then a maintainer would do a push to prod PR, merge that, and it take another 13 minutes to get to the prod website. As maintainers, there are times when we push things all the way to prod quickly, or, there are times when we will control what is going out - this could include release days, an important blog announcement, site changes, etc. Having a prod merge for every merge in main would add levels of confusion non-maintainers likely don't quite see in the normal back-end processes. Also, when there are things on staging that haven't been pushed to prod, the latest push to prod PR picks all of it up. Having all these auto-created push to prod PRs created will result in a mass of duplicated PRs in the repo, adding another layer of complexity and maintenance for maintainers to manage. No while this sounds like an OK idea, it really isn't something we would want to do.

I don't disagree with @krisfreedain, but I still believe the workflow could be useful for small PRs that can be merged and automatically backported to the prod branch. Ultimately, it's the maintainer's choice to add the labels. A maintainer can decide to either apply a backport label or manually create a PR. Currently, maintainers target a series of PRs, collect them (perhaps cherry-picking a commit), and create one PR for prod. In this process, a maintainer could skip a PR if it was auto-created by the backport workflow. During a release, once the PR is merged and we confirm the staging site is accurate, the maintainer can asynchronously add a label to the merged PR, triggering the creation of a backport PR. However the maintainer still has the final decision to add the label during the review.

Adding @dblock @getsaurabh02

@nateynateynate
Copy link
Member

Oh we used to have this? like merging a set of PR's to the prod branch rather than targeting individual PR's?

Normally when I make a PR to prod, I just compare main vs prod and merge the entirety of the difference. It sounds like we still have the option to do that if I'm reading this right.

@dblock
Copy link
Member

dblock commented Sep 30, 2024

[Catch All Triage - 1, 2, 3, 4]

@dblock dblock removed the untriaged label Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🔖 Ready
Development

No branches or pull requests

5 participants