Skip to content

Commit

Permalink
Rollback the logger & try/catch changes from #77640
Browse files Browse the repository at this point in the history 
 * Doing a try/catch and re-throwing doesn't gain us anything. We already catching the error in the route handler
 * We have logging for the issue in the existing handler. We also don't pass a logging context to functions
  • Loading branch information
John Schulz committed Sep 17, 2020
1 parent 12cd179 commit fdcaa3a
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 128 deletions.
201 changes: 93 additions & 108 deletions x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/transform/install.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Logger, SavedObjectsClientContract } from 'kibana/server';
import { SavedObjectsClientContract } from 'kibana/server';

import { saveInstalledEsRefs } from '../../packages/install';
import * as Registry from '../../registry';
Expand Down Expand Up @@ -33,100 +33,92 @@ export const installTransformForDataset = async (
registryPackage: RegistryPackage,
paths: string[],
callCluster: CallESAsCurrentUser,
savedObjectsClient: SavedObjectsClientContract,
logger: Logger
savedObjectsClient: SavedObjectsClientContract
) => {
try {
const installation = await getInstallation({
savedObjectsClient,
pkgName: registryPackage.name,
});
let previousInstalledTransformEsAssets: EsAssetReference[] = [];
if (installation) {
previousInstalledTransformEsAssets = installation.installed_es.filter(
({ type, id }) => type === ElasticsearchAssetType.transform
);
}

// delete all previous transform
await deleteTransforms(
callCluster,
previousInstalledTransformEsAssets.map((asset) => asset.id)
const installation = await getInstallation({
savedObjectsClient,
pkgName: registryPackage.name,
});
let previousInstalledTransformEsAssets: EsAssetReference[] = [];
if (installation) {
previousInstalledTransformEsAssets = installation.installed_es.filter(
({ type, id }) => type === ElasticsearchAssetType.transform
);
// install the latest dataset
const datasets = registryPackage.datasets;
if (!datasets?.length) return [];
const installNameSuffix = `${registryPackage.version}`;

const transformPaths = paths.filter((path) => isTransform(path));
let installedTransforms: EsAssetReference[] = [];
if (transformPaths.length > 0) {
const transformPathDatasets = datasets.reduce<TransformPathDataset[]>((acc, dataset) => {
transformPaths.forEach((path) => {
if (isDatasetTransform(path, dataset.path)) {
acc.push({ path, dataset });
}
});
return acc;
}, []);

const transformRefs = transformPathDatasets.reduce<EsAssetReference[]>(
(acc, transformPathDataset) => {
if (transformPathDataset) {
acc.push({
id: getTransformNameForInstallation(transformPathDataset, installNameSuffix),
type: ElasticsearchAssetType.transform,
});
}
return acc;
},
[]
);

// get and save transform refs before installing transforms
await saveInstalledEsRefs(savedObjectsClient, registryPackage.name, transformRefs);

const transforms: TransformInstallation[] = transformPathDatasets.map(
(transformPathDataset: TransformPathDataset) => {
return {
installationName: getTransformNameForInstallation(
transformPathDataset,
installNameSuffix
),
content: getAsset(transformPathDataset.path).toString('utf-8'),
};
}
);
}

const installationPromises = transforms.map(async (transform) => {
return installTransform({ callCluster, transform, logger });
// delete all previous transform
await deleteTransforms(
callCluster,
previousInstalledTransformEsAssets.map((asset) => asset.id)
);
// install the latest dataset
const datasets = registryPackage.datasets;
if (!datasets?.length) return [];
const installNameSuffix = `${registryPackage.version}`;

const transformPaths = paths.filter((path) => isTransform(path));
let installedTransforms: EsAssetReference[] = [];
if (transformPaths.length > 0) {
const transformPathDatasets = datasets.reduce<TransformPathDataset[]>((acc, dataset) => {
transformPaths.forEach((path) => {
if (isDatasetTransform(path, dataset.path)) {
acc.push({ path, dataset });
}
});
return acc;
}, []);

const transformRefs = transformPathDatasets.reduce<EsAssetReference[]>(
(acc, transformPathDataset) => {
if (transformPathDataset) {
acc.push({
id: getTransformNameForInstallation(transformPathDataset, installNameSuffix),
type: ElasticsearchAssetType.transform,
});
}
return acc;
},
[]
);

installedTransforms = await Promise.all(installationPromises).then((results) =>
results.flat()
);
}
// get and save transform refs before installing transforms
await saveInstalledEsRefs(savedObjectsClient, registryPackage.name, transformRefs);

const transforms: TransformInstallation[] = transformPathDatasets.map(
(transformPathDataset: TransformPathDataset) => {
return {
installationName: getTransformNameForInstallation(
transformPathDataset,
installNameSuffix
),
content: getAsset(transformPathDataset.path).toString('utf-8'),
};
}
);

if (previousInstalledTransformEsAssets.length > 0) {
const currentInstallation = await getInstallation({
savedObjectsClient,
pkgName: registryPackage.name,
});
const installationPromises = transforms.map(async (transform) => {
return installTransform({ callCluster, transform });
});

// remove the saved object reference
await deleteTransformRefs(
savedObjectsClient,
currentInstallation?.installed_es || [],
registryPackage.name,
previousInstalledTransformEsAssets.map((asset) => asset.id),
installedTransforms.map((installed) => installed.id)
);
}
return installedTransforms;
} catch (err) {
logger.error(err);
throw err;
installedTransforms = await Promise.all(installationPromises).then((results) => results.flat());
}

if (previousInstalledTransformEsAssets.length > 0) {
const currentInstallation = await getInstallation({
savedObjectsClient,
pkgName: registryPackage.name,
});

// remove the saved object reference
await deleteTransformRefs(
savedObjectsClient,
currentInstallation?.installed_es || [],
registryPackage.name,
previousInstalledTransformEsAssets.map((asset) => asset.id),
installedTransforms.map((installed) => installed.id)
);
}
return installedTransforms;
};

const isTransform = (path: string) => {
Expand All @@ -147,31 +139,24 @@ const isDatasetTransform = (path: string, datasetName: string) => {
async function installTransform({
callCluster,
transform,
logger,
}: {
callCluster: CallESAsCurrentUser;
transform: TransformInstallation;
logger: Logger;
}): Promise<EsAssetReference> {
try {
// defer validation on put if the source index is not available
await callCluster('transport.request', {
method: 'PUT',
path: `/_transform/${transform.installationName}`,
query: 'defer_validation=true',
body: transform.content,
});

await callCluster('transport.request', {
method: 'POST',
path: `/_transform/${transform.installationName}/_start`,
});

return { id: transform.installationName, type: ElasticsearchAssetType.transform };
} catch (err) {
logger.error(err);
throw err;
}
// defer validation on put if the source index is not available
await callCluster('transport.request', {
method: 'PUT',
path: `/_transform/${transform.installationName}`,
query: 'defer_validation=true',
body: transform.content,
});

await callCluster('transport.request', {
method: 'POST',
path: `/_transform/${transform.installationName}/_start`,
});

return { id: transform.installationName, type: ElasticsearchAssetType.transform };
}

const getTransformNameForInstallation = (
Expand Down
21 changes: 4 additions & 17 deletions x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/transform/transform.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { loggingSystemMock } from '../../../../../../../../src/core/server/logging/logging_system.mock';

jest.mock('../../packages/get', () => {
return { getInstallation: jest.fn(), getInstallationObject: jest.fn() };
});
Expand All @@ -18,12 +15,7 @@ jest.mock('./common', () => {
});

import { installTransformForDataset } from './install';
import {
ILegacyScopedClusterClient,
LoggerFactory,
SavedObject,
SavedObjectsClientContract,
} from 'kibana/server';
import { ILegacyScopedClusterClient, SavedObject, SavedObjectsClientContract } from 'kibana/server';
import { ElasticsearchAssetType, Installation, RegistryPackage } from '../../../../types';
import { getInstallation, getInstallationObject } from '../../packages';
import { getAsset } from './common';
Expand All @@ -33,7 +25,6 @@ import { savedObjectsClientMock } from '../../../../../../../../src/core/server/
describe('test transform install', () => {
let legacyScopedClusterClient: jest.Mocked<ILegacyScopedClusterClient>;
let savedObjectsClient: jest.Mocked<SavedObjectsClientContract>;
let logger: jest.Mocked<LoggerFactory>;
beforeEach(() => {
legacyScopedClusterClient = {
callAsInternalUser: jest.fn(),
Expand All @@ -42,7 +33,6 @@ describe('test transform install', () => {
(getInstallation as jest.MockedFunction<typeof getInstallation>).mockReset();
(getInstallationObject as jest.MockedFunction<typeof getInstallationObject>).mockReset();
savedObjectsClient = savedObjectsClientMock.create();
logger = loggingSystemMock.create();
});

afterEach(() => {
Expand Down Expand Up @@ -142,8 +132,7 @@ describe('test transform install', () => {
'endpoint-0.16.0-dev.0/dataset/metadata_current/elasticsearch/transform/default.json',
],
legacyScopedClusterClient.callAsCurrentUser,
savedObjectsClient,
logger.get('ingest')
savedObjectsClient
);
expect(legacyScopedClusterClient.callAsCurrentUser.mock.calls).toEqual([
[
Expand Down Expand Up @@ -297,8 +286,7 @@ describe('test transform install', () => {
} as unknown) as RegistryPackage,
['endpoint-0.16.0-dev.0/dataset/metadata_current/elasticsearch/transform/default.json'],
legacyScopedClusterClient.callAsCurrentUser,
savedObjectsClient,
logger.get('ingest')
savedObjectsClient
);

expect(legacyScopedClusterClient.callAsCurrentUser.mock.calls).toEqual([
Expand Down Expand Up @@ -395,8 +383,7 @@ describe('test transform install', () => {
} as unknown) as RegistryPackage,
[],
legacyScopedClusterClient.callAsCurrentUser,
savedObjectsClient,
logger.get('ingest')
savedObjectsClient
);

expect(legacyScopedClusterClient.callAsCurrentUser.mock.calls).toEqual([
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { deleteKibanaSavedObjectsAssets } from './remove';
import { PackageOutdatedError } from '../../../errors';
import { getPackageSavedObjects } from './get';
import { installTransformForDataset } from '../elasticsearch/transform/install';
import { appContextService } from '../../app_context';

export async function installLatestPackage(options: {
savedObjectsClient: SavedObjectsClientContract;
Expand Down Expand Up @@ -197,8 +196,7 @@ export async function installPackage({
registryPackageInfo,
paths,
callCluster,
savedObjectsClient,
appContextService.getLogger()
savedObjectsClient
);

// if this is an update or retrying an update, delete the previous version's pipelines
Expand Down

0 comments on commit fdcaa3a

Please sign in to comment.