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

fix: revert lookup refactor from #32930 #33184

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/util/promises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function isExternalHostError(err: any): err is ExternalHostError {
return err instanceof ExternalHostError;
}

export function handleMultipleErrors(errors: Error[]): never {
function handleMultipleErrors(errors: Error[]): never {
const hostError = errors.find(isExternalHostError);
if (hostError) {
throw hostError;
Expand Down
8 changes: 2 additions & 6 deletions lib/workers/repository/process/fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { getConfig } from '../../../config/defaults';
import { MavenDatasource } from '../../../modules/datasource/maven';
import type { PackageFile } from '../../../modules/manager/types';
import { ExternalHostError } from '../../../types/errors/external-host-error';
import { Result } from '../../../util/result';
import { fetchUpdates } from './fetch';
import * as lookup from './lookup';

Expand Down Expand Up @@ -59,21 +58,18 @@ describe('workers/repository/process/fetch', () => {
depName: 'abcd',
packageName: 'abcd',
skipReason: 'ignored',
skipStage: 'lookup',
updates: [],
},
{
depName: 'foo',
packageName: 'foo',
skipReason: 'disabled',
skipStage: 'lookup',
updates: [],
},
{
depName: 'skipped',
packageName: 'skipped',
skipReason: 'some-reason',
skipStage: 'lookup',
updates: [],
},
],
Expand Down Expand Up @@ -201,7 +197,7 @@ describe('workers/repository/process/fetch', () => {
},
],
};
lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
lookupUpdates.mockRejectedValueOnce(new Error('some error'));

await expect(
fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),
Expand All @@ -218,7 +214,7 @@ describe('workers/repository/process/fetch', () => {
},
],
};
lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
lookupUpdates.mockRejectedValueOnce(new Error('some error'));

await expect(
fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),
Expand Down
129 changes: 48 additions & 81 deletions lib/workers/repository/process/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ import type { LookupUpdateConfig, UpdateResult } from './lookup/types';

type LookupResult = Result<PackageDependency, Error>;

interface LookupTaskResult {
packageFile: PackageFile;
result: LookupResult;
}

type LookupTask = Promise<LookupTaskResult>;

async function lookup(
packageFileConfig: RenovateConfig & PackageFile,
indep: PackageDependency,
Expand Down Expand Up @@ -113,89 +106,63 @@ async function lookup(
});
}

function createLookupTasks(
async function fetchManagerPackagerFileUpdates(
config: RenovateConfig,
managerPackageFiles: Record<string, PackageFile[]>,
): LookupTask[] {
const lookupTasks: LookupTask[] = [];

for (const [manager, packageFiles] of Object.entries(managerPackageFiles)) {
const managerConfig = getManagerConfig(config, manager);

for (const packageFile of packageFiles) {
const packageFileConfig = mergeChildConfig(managerConfig, packageFile);
if (packageFile.extractedConstraints) {
packageFileConfig.constraints = {
...packageFile.extractedConstraints,
...config.constraints,
};
}

for (const dep of packageFile.deps) {
lookupTasks.push(
lookup(packageFileConfig, dep).then((result) => ({
packageFile,
result,
})),
);
}
}
managerConfig: RenovateConfig,
pFile: PackageFile,
): Promise<void> {
const { packageFile } = pFile;
const packageFileConfig = mergeChildConfig(managerConfig, pFile);
if (pFile.extractedConstraints) {
packageFileConfig.constraints = {
...pFile.extractedConstraints,
...config.constraints,
};
}
const { manager } = packageFileConfig;
const queue = pFile.deps.map(
(dep) => async (): Promise<PackageDependency> => {
const updates = await lookup(packageFileConfig, dep);
return updates.unwrapOrThrow();
},
);
logger.trace(
{ manager, packageFile, queueLength: queue.length },
'fetchManagerPackagerFileUpdates starting with concurrency',
);

return lookupTasks;
pFile.deps = await p.all(queue);
logger.trace({ packageFile }, 'fetchManagerPackagerFileUpdates finished');
}

export async function fetchUpdates(
async function fetchManagerUpdates(
config: RenovateConfig,
managerPackageFiles: Record<string, PackageFile[]>,
packageFiles: Record<string, PackageFile[]>,
manager: string,
): Promise<void> {
logger.debug(
{ baseBranch: config.baseBranch },
'Starting package releases lookups',
const managerConfig = getManagerConfig(config, manager);
const queue = packageFiles[manager].map(
(pFile) => (): Promise<void> =>
fetchManagerPackagerFileUpdates(config, managerConfig, pFile),
);
logger.trace(
{ manager, queueLength: queue.length },
'fetchManagerUpdates starting',
);
await p.all(queue);
logger.trace({ manager }, 'fetchManagerUpdates finished');
}

const allTasks = createLookupTasks(config, managerPackageFiles);

const fetchResults = await Promise.all(allTasks);

const errors: Error[] = [];

type PackageDeps = WeakMap<PackageFile, PackageDependency[]>;
const packageDeps: PackageDeps = new WeakMap();

// Separate good results from errors
for (const { packageFile, result } of fetchResults) {
const { val: dep, err } = result.unwrap();
if (dep) {
let deps = packageDeps.get(packageFile);
if (!deps) {
deps = [];
packageDeps.set(packageFile, deps);
}
if (dep.skipReason && !dep.skipStage) {
dep.skipStage = 'lookup';
}
deps.push(dep);
} else {
errors.push(err);
}
}

if (errors.length) {
p.handleMultipleErrors(errors);
}

// Assign fetched deps back to packageFiles
for (const packageFiles of Object.values(managerPackageFiles)) {
for (const packageFile of packageFiles) {
const packageFileDeps = packageDeps.get(packageFile);
if (packageFileDeps) {
packageFile.deps = packageFileDeps;
}
}
}

PackageFiles.add(config.baseBranch!, { ...managerPackageFiles });
export async function fetchUpdates(
config: RenovateConfig,
packageFiles: Record<string, PackageFile[]>,
): Promise<void> {
const managers = Object.keys(packageFiles);
const allManagerJobs = managers.map((manager) =>
fetchManagerUpdates(config, packageFiles, manager),
);
await Promise.all(allManagerJobs);
PackageFiles.add(config.baseBranch!, { ...packageFiles });
logger.debug(
{ baseBranch: config.baseBranch },
'Package releases lookups complete',
Expand Down
Loading