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

feat(platform/gitlab): stabilize PR/MR auto-merge for Gitlab #27356

Merged
merged 8 commits into from
Feb 22, 2024

Conversation

afrimberger
Copy link
Contributor

@afrimberger afrimberger commented Feb 16, 2024

Changes

This change is only related to the auto-merge functionality of the Gitlab platform.

Before executing the auto-merge the code checks for various statuses to reduce the number of retries and thus lowering the waiting time.

The actual call to enable auto-merge is now covered by a retry logic. This is needed, because Gitlab sometimes responds with a "405 method not allowed" even though the merge request status is already proposing a merge-able status. The time to consistency window seems to vary depending on the actual load of the Gitlab instance.

The change removes the hard coded check for the Gitlab version introduced with #26438. merge_status is deprecated with Gitlab 15.6 and will be removed sooner or later in favor of detailed_merge_status. Hence, detailed_merge_status should be preferred if available.

Context

I'm executing Renovate Bot across ~150 repos and observed many unmerged merge requests even though the build succeeded. With a high change rate of the libraries, the MRs tend to stay open for too long. Hence, I wanted to improve the stability of the auto-merge with the current Renovate Bot run.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@CLAassistant
Copy link

CLAassistant commented Feb 16, 2024

CLA assistant check
All committers have signed the CLA.

@rarkins
Copy link
Collaborator

rarkins commented Feb 16, 2024

@javaxiss could you take a look if you have time?

lib/modules/platform/gitlab/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
@afrimberger
Copy link
Contributor Author

@viceice : All comments are resolved. Is there anything else left or can we get this PR merged?

@viceice
Copy link
Member

viceice commented Feb 21, 2024

@viceice : All comments are resolved. Is there anything else left or can we get this PR merged?

yes, failed tests.

@afrimberger
Copy link
Contributor Author

@viceice : All comments are resolved. Is there anything else left or can we get this PR merged?

yes, failed tests.

The failing test is totally unrelated to my changes. It's located in lib/config/decrypt.spec.ts (I changed only platform/gitlab:

FAIL lib/config/decrypt.spec.ts (6.15 s, 163 MB heap size)
  ● config/decrypt › decryptConfig() › handles PGP org constraint

    config-validation

      190 |           }
      191 |           if (!is.nonEmptyString(decryptedStr)) {
    > 192 |             const error = new Error('config-validation');
          |                           ^
      193 |             error.validationError = `Failed to decrypt field ${eKey}. Please re-encrypt and try again.`;
      194 |             throw error;
      195 |           }

      at decryptConfig (lib/config/decrypt.ts:192:27)
      at Object.<anonymous> (lib/config/decrypt.spec.ts:156:19)

My PR doesn't introduce any config or PGP related changes. Furthermore, all tests are locally green. Especially the one raised by the build pipeline.

image

@viceice or @rarkins: Is it maybe a flaky test?

@rarkins
Copy link
Collaborator

rarkins commented Feb 21, 2024

I've updated the branch from main again to see the results

@rarkins rarkins requested a review from viceice February 21, 2024 16:40
@rarkins
Copy link
Collaborator

rarkins commented Feb 21, 2024

It's passing now

viceice
viceice previously approved these changes Feb 21, 2024
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
auto-merge was automatically disabled February 22, 2024 08:47

Head branch was pushed to by a user without write access

@afrimberger
Copy link
Contributor Author

@viceice would you mind re-approving the MR? Thanks! :)

@rarkins rarkins requested a review from viceice February 22, 2024 08:57
@rarkins rarkins added this pull request to the merge queue Feb 22, 2024
Merged via the queue into renovatebot:main with commit 1f8e535 Feb 22, 2024
35 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.209.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants