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(gitlab): add branch status check attemps #32692

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

philippe-granet
Copy link

@philippe-granet philippe-granet commented Nov 23, 2024

Changes

  • Add branch status check attemps before trying to add status check
  • Allow overriding the branch status check attempts via experimental env RENOVATE_X_GITLAB_BRANCH_STATUS_CHECK_ATTEMPS.

Context

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

@philippe-granet philippe-granet marked this pull request as ready for review November 24, 2024 11:37
docs/usage/self-hosted-experimental.md Outdated Show resolved Hide resolved
docs/usage/self-hosted-experimental.md Outdated Show resolved Hide resolved
lib/modules/platform/gitlab/index.ts Outdated Show resolved Hide resolved
@rarkins
Copy link
Collaborator

rarkins commented Nov 27, 2024

Could this instead be default functionality, with a large number of attempts, e.g. 10?

const delay = 1000;
const retry = 5;
process.env.RENOVATE_X_GITLAB_BRANCH_STATUS_CHECK_ATTEMPTS =
String(retry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
String(retry);
`${retry}`;

use string template or toString method

delete it in beforeEach to not influence other tests

2,
);

for (let attempt = 1; attempt <= retryTimes; attempt += 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole loop should have a try catch and early return on failures from gitlab API

@philippe-granet
Copy link
Author

philippe-granet commented Nov 27, 2024

Could this instead be default functionality, with a large number of attempts, e.g. 10?

This will penalize projects that do not have defined pipelines, this will systematically wait 10s and slow down the renovate analyzes

@rarkins
Copy link
Collaborator

rarkins commented Nov 27, 2024

Could this instead be default functionality, with a large number of attempts, e.g. 10?

This will penalize projects that do not have defined pipelines, this will systematically wait 10s and slow down the renovate analyzes

Thanks for the info. I think I missed this from the various conversations before. Could make sure this info of when this is setting is applicable and when it's not is captured in the markdown doc in this PR? I will then decide if it needs to be a first-class option or brainstorm if we have a clever way to detect this. If this is a common problem we prefer that users don't need to set obscure experimental options.

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.

4 participants