Perform an API call to check if Renovate is enabled, rather than cloning the repo #18811
Comments
jamietanna
added
priority-5-triage
status:requirements
|
if you want to skip repos via API, add a renovate.json with enabled=false https://docs.renovatebot.com/self-hosted-configuration/#optimizefordisabled |
|
Interesting - would this still require each repo to be "onboarded" with a |
|
This behaviour could probably be tweaked to behave differently when onboarding=false and requireConfig=optional like you have it. As long as you are ok with using renovate.json as the file name |
|
Nice, so when I run: env RENOVATE_REQUIRE_CONFIG=required RENOVATE_ONBOARDING=false renovate --token $GITHUB_TOKEN --autodiscover-filter 'jamietanna/*' --autodiscover --optimize-for-disabledI get: Full logMaybe I'm misunderstanding, but I'd expect that in this case, we wouldn't clone after seeing there's no |
|
renovate clones, because it needs to check some more possible config files renovate/lib/config/app-strings.ts Lines 1 to 11 in d869c94 so you should enable repo cache and save |
|
Gotcha - do we think it'd be worth an enhancement to do all of those checks via API calls, instead of cloning to check? So when running: env RENOVATE_REPOSITORY_CACHE=enabled LOG_LEVEL=debug RENOVATE_REQUIRE_CONFIG=required RENOVATE_ONBOARDING=false renovate --token $GITHUB_TOKEN --autodiscover-filter 'jamietanna/*' --autodiscover --optimize-for-disabled --base-dir /tmp/renovate-base-new --cache-dir /tmp/renovate-cache-new --dry-run The result of |
|
Looking at a significantly larger project, when running: env RENOVATE_REPOSITORY_CACHE=enabled LOG_LEVEL=debug RENOVATE_REQUIRE_CONFIG=optional RENOVATE_ONBOARDING=false renovate --token $GITHUB_TOKEN --autodiscover-filter '...' --autodiscover --optimize-for-disabled --base-dir /tmp/renovate-base-new --cache-dir /tmp/renovate-cache-new --dry-runThen the result of |
|
I was meaning we could tweak optimizeForDisabled |
|
I'm not sure repoCache helps here because we'd still need to clone any time there's a new commit to make sure no config files. I think the optimization can work as long as you're happy to use only renovate.json |
|
ok. so we can assume disabled if onboarding is false, required config is true and we get a 404 for the default config file ( |
|
Nice, yeah that'd work 👍 In our current setup, it wouldn't save us any time, as we're having to pre-filter the organisations' repos and split them into groups of 100, because we're hitting renovatebot/github-action#648 with our GitHub App authentication |
|
I am hitting the same issue. We have hundreds of internal projects and cloning them all takes a very long time. The relevant part of my config looks like this: autodiscover: true
optimizeForDisabled: true
requireConfig: 'required'
onboarding: falseThe documentation of
I think this can be improved by also skipping a repo when I am mainly a Java developer, so I unfortunately cannot provide a PR for this. But anyway, would a PR that implements this proposal even be considered for merging? Or are there any potential issues with this that I am overlooking? |
|
The code in question seems to be The heavy lifting seems to be already done. I guess all that needs to be done is to slightly extend the method to include an additional check, like so: async function validateOptimizeForDisabled(
config: RenovateConfig
): Promise<void> {
if (config.optimizeForDisabled) {
const renovateConfig = await getJsonFile(defaultConfigFile(config));
if (renovateConfig == null && !config.onboarding && config.requireConfig == 'required') {
throw new Error(REPOSITORY_NO_CONFIG);
}
if (renovateConfig?.enabled === false) {
throw new Error(REPOSITORY_DISABLED_BY_CONFIG);
}
}
}This seems simple enough, even for me. Should I try to submit a PR for that? |
|
sure, but probably needs at least a new test for code coverage. |
|
I don't know how to write unit tests in TypeScript, but I could probably use an existing one as a template. If anyone else wants to do it, feel free not to wait for me! |
|
So we are adding a special case, where all these are true?
In this case the repo must use |
|
This should be rushed for v35 or wait for v36, as it's technically a breaking change |
rarkins
added
breaking
|
@TWith2Sugars is this one you may be able to help contribute? |
|
@rarkins Could you please explain why this is a breaking change in your eyes? I cannot imagine an example where this is anything else than a performance optimization. Also, when |
|
It's a breaking change because if someone has a repo with a config file like |
|
@rarkins Thanks, that's right! I didn't see that. In other words, currently In this case, I am totally fine with delaying this until one of the next major releases. Not only because it is breaking, but also to give us more time to think about it and maybe come up with a better solution. |
|
Maybe this can made less breaking by only skipping a repo if all of the criteria mentioned in #18811 (comment) are true AND also only if I guess that the combination of Anyway: I think it is reasonable to only perform this optimization if |
|
Either way it's breaking, so I wouldn't go too far out of your way to accommodate it. The combination of configuration options is already specific enough that I think it's ok to say "if you use optimizeForDisabled=true and onboarding=false, you must use |
|
Agreed, with the exception that I wouldn't hardcode |
|
I tried enabling the same configuration as specified in #18811 (comment) today to speed up our executions as we have thousands of repositories taking hours to run, so this would be an incredibly welcomed (and timely) change. We are just now rolling out Renovate internally so we only have dozens of repositories with it enabled at this time. |
|
@jamietanna Do you like to provide a PR for v36? Should be done in next weeks of cause 🙃 |
|
I'd be up for this, but may not get to it for a week or so so if anyone else on this issue is interested in claiming it, go ahead! |
|
OK, will add it to the planning. We can drop it if it's not ready 😉 |
|
Recent versions of Renovate include an onboarding cache: I may be missing something, but can this cache be extended to severely improve the situation for non-active repos when This wouldn't not even be breaking, in contrast to the changes suggested above. |
|
It could in theory yes |
The cache is for onboarding branch. In your case there is no onboarding branch so this wouldn't work as it is. |
|
I've implemented a workaround that works extremely well for us. Since it may be useful for others, I've decided to share it here. My workaround makes use of the fact that the configuration can be a script file ( TL;DR, this is my const gitlabApiEndpoint = 'https://our.company.gitlab/api/v4'
const {logger: logger} = require('renovate/dist/logger');
const gitlab = require('renovate/dist/util/http/gitlab');
gitlab.setBaseUrl(gitlabApiEndpoint);
const {default: PQueue} = require('p-queue');
const httpOpts = {
paginate: true,
token: process.env.RENOVATE_TOKEN
};
const http = new gitlab.GitlabHttp('gitlab', httpOpts);
async function toNullIfRenovateDisabled(project) {
const pId = project.id;
const ref = project.default_branch;
try {
await http.head(`projects/${pId}/repository/files/renovate.json?ref=${ref}`);
return project;
} catch (err) {
if (err.statusCode === 404 || err.statusCode === 403) {
return null;
}
throw new Error("Couldn't retrieve renovate.json", { cause: err });
}
}
async function retrieveProjects() {
try {
const response = await http.getJson(`projects?per_page=100&archived=false&membership=true&min_access_level=30`, httpOpts);
return response.body.filter(p => !p.mirror && !p.marked_for_deletion_on);
} catch (err) {
throw new Error("Couldn't retrieve projects", { cause: err });
}
}
function toRenovateRepositories(projects) {
return projects.map(p => p.path_with_namespace);
}
async function getRepositories() {
const projects = await retrieveProjects();
const enabledProjects = [];
const queue = new PQueue({concurrency: 10});
for (const p of projects) {
queue.add(() => toNullIfRenovateDisabled(p).then((p) => {
if (p) {
enabledProjects.push(p);
}
}));
}
await queue.onIdle();
return toRenovateRepositories(enabledProjects);
}
module.exports = async function() {
const enabledRepos = await getRepositories();
logger.info({ repositories: enabledRepos }, 'Detected repositories:');
return {
platform: 'gitlab',
endpoint: gitlabApiEndpoint,
autodiscover: false,
requireConfig: 'optional', // performance opt: Skip checking for closed PRs
onboarding: false,
repositories: enabledRepos,
repositoryCache: 'enabled' // experimental flag
};
}Disclaimer and caveats:
Anyway, this cuts the time of a Renovate execution by more than 70% for us, saves tons of traffic and surely reduces the load on our Gitlab instance. I am very happy with this result. |
|
Thanks @ChristianCiach |
|
Hi, this issue got quite long with 33 comments, making it hard to know what the "real" feature to implement is. If anyone still wants this, please suggest a feature in Discussions with a concise summary of desired behavior, and once it's settled then we'll create a new Feature Request issue. |
and others
What would you like Renovate to be able to do?
Related to #18739 and #18740, when running against ~2000 repos, cloning each repo to then find out that there's no
renovate.jsonor similar can be costly.If we instead - through some configuration - could decide to only check via an API call, rather than checking out the full repo, we could save significant bandwidth and time.
If you have any ideas on how this should be implemented, please tell us here.
RENOVATE_PRE_CHECK_DEFAULT_BRANCHrenovate.json(or other filenames)Is this a feature you are interested in implementing yourself?
Yes
The text was updated successfully, but these errors were encountered: