Skip to content

Commit

Permalink
PR feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
awahab07 committed Jan 5, 2024
1 parent 12aa709 commit 02bfbe0
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export async function getDataStreamDetails(args: {

const indexSettings = await dataStreamService.getDataSteamIndexSettings(esClient, {
type,
dataset: `*${datasetQuery}*`,
dataset: `${datasetQuery}`,
});

const indexesList = Object.values(indexSettings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import * as t from 'io-ts';
import { keyBy, merge, values } from 'lodash';
import Boom from '@hapi/boom';
import {
DataStreamDetails,
DataStreamStat,
Expand Down Expand Up @@ -113,7 +114,7 @@ const dataStreamDetailsRoute = createDatasetQualityServerRoute({
tags: [],
},
async handler(resources): Promise<DataStreamDetails> {
const { context, params, response } = resources;
const { context, params } = resources;
const { type = DEFAULT_DATASET_TYPE, datasetQuery } = params.query;
const coreContext = await context.core;

Expand All @@ -125,9 +126,7 @@ const dataStreamDetailsRoute = createDatasetQualityServerRoute({
} catch (e) {
if (e) {
if (e?.message?.indexOf('index_not_found_exception') > -1) {
throw response.notFound({
body: { message: `Data stream ${type}-*${datasetQuery}* not found.` },
});
throw Boom.notFound(`Data stream ${type}-${datasetQuery} not found.`);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/
import { errors } from '@elastic/elasticsearch';
import Boom from '@hapi/boom';
import { isKibanaResponse } from '@kbn/core-http-router-server-internal';
import { CoreSetup, Logger, RouteRegistrar } from '@kbn/core/server';
import {
ServerRouteRepository,
Expand Down Expand Up @@ -54,7 +53,6 @@ export function registerRoutes({ repository, core, logger, plugins }: RegisterRo
const data = (await handler({
context,
request,
response,
logger,
params: decodedParams,
plugins,
Expand All @@ -73,11 +71,8 @@ export function registerRoutes({ repository, core, logger, plugins }: RegisterRo
body: { message: error.output.payload.message },
});
}
logger.error(error);

if (isKibanaResponse(error)) {
return error;
}
logger.error(error);

const opts = {
statusCode: 500,
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/dataset_quality/server/routes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { KibanaRequest, KibanaResponseFactory, Logger } from '@kbn/core/server';
import { KibanaRequest, Logger } from '@kbn/core/server';
import { DatasetQualityServerRouteRepository } from '.';
import {
DatasetQualityPluginSetupDependencies,
Expand All @@ -18,7 +18,6 @@ export interface DatasetQualityRouteHandlerResources {
context: DatasetQualityRequestHandlerContext;
logger: Logger;
request: KibanaRequest;
response: KibanaResponseFactory;
plugins: {
[key in keyof DatasetQualityPluginSetupDependencies]: {
setup: Required<DatasetQualityPluginSetupDependencies>[key];
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/dataset_quality/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
"@kbn/es-types",
"@kbn/deeplinks-observability",
"@kbn/router-utils",
"@kbn/core-http-router-server-internal",
],
"exclude": [
"target/**/*",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/test/dataset_quality_api_integration/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
} from '@kbn/dataset-quality-plugin/server/test_helpers/create_dataset_quality_users/authentication';
import { createDatasetQualityUsers } from '@kbn/dataset-quality-plugin/server/test_helpers/create_dataset_quality_users';
import { FtrConfigProviderContext } from '@kbn/test';
import { Client } from '@elastic/elasticsearch';
import supertest from 'supertest';
import { format, UrlObject } from 'url';
import { createLogger, LogLevel, LogsSynthtraceEsClient } from '@kbn/apm-synthtrace';
Expand Down Expand Up @@ -66,7 +65,6 @@ export interface CreateTest {
services: InheritedServices & {
datasetQualityFtrConfig: () => DatasetQualityFtrConfig;
registry: ({ getService }: FtrProviderContext) => ReturnType<typeof RegistryProvider>;
es: (context: InheritedFtrProviderContext) => Client;
logSynthtraceEsClient: (
context: InheritedFtrProviderContext
) => Promise<LogsSynthtraceEsClient>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
const start = '2023-12-11T18:00:00.000Z';
const end = '2023-12-11T18:01:00.000Z';
const dataStream = 'nginx.access';
const namespace = 'default';

async function callApiAs(user: DatasetQualityApiClientKey, datasetQuery: string) {
return await datasetQualityApiClient[user]({
Expand All @@ -44,6 +45,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
.message('This is a log message')
.timestamp(timestamp)
.dataset(dataStream)
.namespace(namespace)
.defaults({
'log.file.path': '/my-service.log',
})
Expand All @@ -59,20 +61,20 @@ export default function ApiTest({ getService }: FtrProviderContext) {

it('returns 404 if matching data stream is not available', async () => {
const nonExistentDataStream = 'Non-existent';
const expectedMessage = `*${nonExistentDataStream}* not found`;
expect(
(
await expectToReject(() => callApiAs('datasetQualityLogsUser', nonExistentDataStream))
).message.indexOf(expectedMessage)
).to.greaterThan(-1);
const expectedMessage = `logs-${nonExistentDataStream}-${namespace} not found`;
const err = await expectToReject(() =>
callApiAs('datasetQualityLogsUser', `${nonExistentDataStream}-${namespace}`)
);
expect(err.res.status).to.be(404);
expect(err.res.body.message.indexOf(expectedMessage)).to.greaterThan(-1);
});

it('returns data stream details correctly', async () => {
const dataStreamSettings = await getDataStreamSettingsOfFirstIndex(
esClient,
`logs-*${dataStream}*`
`logs-${dataStream}-${namespace}`
);
const resp = await callApiAs('datasetQualityLogsUser', dataStream);
const resp = await callApiAs('datasetQualityLogsUser', `${dataStream}-${namespace}`);
expect(resp.body.createdOn).to.be(Number(dataStreamSettings?.index?.creation_date));
});

Expand Down
69 changes: 0 additions & 69 deletions x-pack/test/dataset_quality_api_integration/utils/data_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,75 +6,6 @@
*/

import { Client } from '@elastic/elasticsearch';
import { IndicesPutIndexTemplateRequest } from '@elastic/elasticsearch/lib/api/types';

export async function createIndexTemplateAndDataStream(
es: Client,
name: string,
indexTemplateOverrides?: Partial<IndicesPutIndexTemplateRequest>
) {
// A data stream requires an index template before it can be created.
await es.indices.putIndexTemplate({
name: `${name}_index_template`,
index_patterns: [name + '*'],
data_stream: {},
...(indexTemplateOverrides ?? {}),
template: {
mappings: {
...(indexTemplateOverrides?.template?.mappings ?? {}),
properties: indexTemplateOverrides?.template?.mappings?.properties ?? {
'@timestamp': {
type: 'date',
},
source: {
type: 'keyword',
},
reference: {
type: 'keyword',
},
params: {
enabled: false,
type: 'object',
},
host: {
properties: {
hostname: {
type: 'text',
fields: {
keyword: {
type: 'keyword',
ignore_above: 256,
},
},
},
id: {
type: 'keyword',
},
name: {
type: 'keyword',
},
},
},
},
},
},
});

await es.indices.createDataStream({ name });
}

async function deleteComposableIndexTemplate(es: Client, name: string) {
await es.indices.deleteIndexTemplate({ name });
}

export async function deleteIndexTemplateAndDataStream(
es: Client,
name: string,
indexTemplateName?: string
) {
await es.indices.deleteDataStream({ name });
await deleteComposableIndexTemplate(es, indexTemplateName ?? `${name}_index_template`);
}

export async function getDataStreamSettingsOfFirstIndex(es: Client, name: string) {
const matchingIndexesObj = await es.indices.getSettings({ index: name });
Expand Down

0 comments on commit 02bfbe0

Please sign in to comment.