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(onboarding): skip processing onboarding branch #22490

Merged

Conversation

RahulGautamSingh
Copy link
Collaborator

@RahulGautamSingh RahulGautamSingh commented May 30, 2023

Changes

  • added new variable onboardingCacheValid to the OnboardingState class which is computed at the beginning and is used to determine if the onboardingCache is valid
  • added two new fields to the OnboardingBranchCache : configFileName and configFileParsed
  • If onboardingCache is valid we reuse this stored info to prevent cloning to find the renovate config file name and its content
    Else onboarding branch is processed and the configFileName and configFileParsed are cached to be used on next run

Context

The GitHub app has something like 50k onboarding repos. So we need some optimizations to reduce the amount of time spent in processing them.
I think if we skip work if main/onboarding branch SHAs are untouched then it’s quick enough
Slack Message @rarkins

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

@RahulGautamSingh RahulGautamSingh marked this pull request as ready for review May 30, 2023 04:23
lib/workers/repository/init/merge.ts Outdated Show resolved Hide resolved
lib/workers/repository/init/merge.ts Outdated Show resolved Hide resolved
lib/workers/repository/onboarding/branch/index.ts Outdated Show resolved Hide resolved
@ChristianCiach
Copy link
Contributor

ChristianCiach commented May 30, 2023

Does this help (or make things more complicated) regarding the optimization I've proposed here?

My use-case consists of tons of repos that do not contain a renovate.json at all. Even though renovate is configured with onboarding: false, all the repos are cloned on every run. My idea is to use the onboarding-cache to skip repos when onboarding: false and we already know that the current branch revision does not contain a renovate.json.

@RahulGautamSingh
Copy link
Collaborator Author

RahulGautamSingh commented May 30, 2023

Does this help (or make things more complicated) regarding the optimization I've proposed here?

My use-case consists of tons of repos that do not contain a renovate.json at all. Even though renovate is configured with onboarding: false, all the repos are cloned on every run. My idea is to use the onboarding-cache to skip repos when onboarding: false and we already know that the current branch revision does not contain a renovate.json.

This PR won't affect your use case since this logic only comes in effect once we know that repoIsOnboarded=false.

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

viceice
viceice previously approved these changes Jun 6, 2023
@RahulGautamSingh RahulGautamSingh requested review from viceice and removed request for viceice June 13, 2023 19:23
@rarkins rarkins added this pull request to the merge queue Jun 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 16, 2023
@rarkins rarkins added this pull request to the merge queue Jun 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 16, 2023
@rarkins
Copy link
Collaborator

rarkins commented Jun 16, 2023

Coverage problem in workers-1 shard

@viceice
Copy link
Member

viceice commented Jun 16, 2023

maybe admin merge the lcov change and clear the codecov data

@rarkins
Copy link
Collaborator

rarkins commented Jun 17, 2023

Needs coverage

lib/workers/repository/init/merge.spec.ts Outdated Show resolved Hide resolved
lib/workers/repository/init/merge.spec.ts Outdated Show resolved Hide resolved
lib/workers/repository/init/merge.ts Outdated Show resolved Hide resolved
@rarkins rarkins added this pull request to the merge queue Jun 21, 2023
Merged via the queue into renovatebot:main with commit fa6e5df Jun 21, 2023
36 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 35.139.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 22, 2023
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.

None yet

5 participants