From 28fd9bc6039e907c4a0fa57188d23f099ef5a7f3 Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Thu, 13 Jan 2022 16:04:30 +0100 Subject: [PATCH] Modify based on review comments. Make it breaking change. Signed-off-by: Jussi Hallila --- .changeset/six-coins-admire.md | 25 ++++++++++++++++--- packages/backend/src/plugins/techInsights.ts | 18 ++++++++----- plugins/tech-insights-backend/README.md | 4 +-- .../src/service/fact/createFactRetriever.ts | 16 ++++++++---- .../src/service/fact/factRetrievers/utils.ts | 12 ++++++--- .../persistence/TechInsightsDatabase.test.ts | 10 ++++---- .../persistence/TechInsightsDatabase.ts | 13 +++++----- plugins/tech-insights-node/src/facts.ts | 14 +++++------ 8 files changed, 72 insertions(+), 40 deletions(-) diff --git a/.changeset/six-coins-admire.md b/.changeset/six-coins-admire.md index 42d0ecbb8f..f2ef03fe01 100644 --- a/.changeset/six-coins-admire.md +++ b/.changeset/six-coins-admire.md @@ -1,12 +1,29 @@ --- -'@backstage/plugin-tech-insights-backend': patch -'@backstage/plugin-tech-insights-node': patch +'@backstage/plugin-tech-insights-backend': minor +'@backstage/plugin-tech-insights-node': minor --- -Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will keep only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. +BREAKING CHANGES: + +- The helper function to create a fact retriever registration is now expecting an object of configuration items instead of individual arguments. + Modify your techInsights.ts plugin configuration in `packages/backend/src/plugins/techInsights.ts` (or equivalent) the following way: + +```diff +-createFactRetrieverRegistration( +- '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. +- entityOwnershipFactRetriever, +-), ++createFactRetrieverRegistration({ ++ cadende: '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. ++ factRetriever: entityOwnershipFactRetriever, ++}), + +``` + +Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'max items' or 'time-to-live'. The former will keep only n number of items in the database for each fact per entity. The latter will remove all facts that are older than the TTL value. Possible values: -- `{ itl: 5 }` // Deletes all facts for the retriever/entity pair, apart from the last five +- `{ maxItems: 5 }` // Deletes all facts for the retriever/entity pair, apart from the last five - `{ ttl: 1209600000 }` // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair - `{ ttl: { weeks: 2 } }` // Deletes all facts older than 2 weeks for the retriever/entity pair diff --git a/packages/backend/src/plugins/techInsights.ts b/packages/backend/src/plugins/techInsights.ts index a05ddc339a..7b0ded2240 100644 --- a/packages/backend/src/plugins/techInsights.ts +++ b/packages/backend/src/plugins/techInsights.ts @@ -40,12 +40,18 @@ export default async function createPlugin({ database, discovery, factRetrievers: [ - createFactRetrieverRegistration( - '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. - entityOwnershipFactRetriever, - ), - createFactRetrieverRegistration('1 1 1 * *', entityMetadataFactRetriever), - createFactRetrieverRegistration('1 1 1 * *', techdocsFactRetriever), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. + factRetriever: entityOwnershipFactRetriever, + }), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', + factRetriever: entityMetadataFactRetriever, + }), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', + factRetriever: techdocsFactRetriever, + }), ], factCheckerFactory: new JsonRulesEngineFactCheckerFactory({ checks: [ diff --git a/plugins/tech-insights-backend/README.md b/plugins/tech-insights-backend/README.md index c0d903b2eb..d78048ef68 100644 --- a/plugins/tech-insights-backend/README.md +++ b/plugins/tech-insights-backend/README.md @@ -91,10 +91,10 @@ const myFactRetrieverRegistration = createFactRetrieverRegistration( ); ``` -FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either ITL (items to live) or TTL (time to live). Valid options for this value are either a number for ITL or a Luxon duration like object for TTL. For example: +FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either MaxItems or TTL (time to live). Valid options for this value are either a number for MaxItems or a Luxon duration like object for TTL. For example: ```ts -const itl = { itl: 7 }; // Deletes all but 7 latest facts for each id/entity pair +const maxItems = { maxItems: 7 }; // Deletes all but 7 latest facts for each id/entity pair const ttl = { ttl: 1209600000 }; // (2 weeks) Deletes items older than 2 weeks const ttlWithAHumanReadableValue = { ttl: { weeks: 2 } }; // Deletes items older than 2 weeks ``` diff --git a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts index 1e3f7f8904..eae579b49f 100644 --- a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts +++ b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts @@ -19,6 +19,12 @@ import { FactRetrieverRegistration, } from '@backstage/plugin-tech-insights-node'; +export type FactRetrieverRegistrationOptions = { + cadence: string; + factRetriever: FactRetriever; + lifecycle?: FactLifecycle; +}; + /** * @public * @@ -47,11 +53,11 @@ import { * \{ itl: 7 \} -- This fact retriever will leave 7 newest items in the database when it is run * */ -export function createFactRetrieverRegistration( - cadence: string, - factRetriever: FactRetriever, - lifecycle?: FactLifecycle, -): FactRetrieverRegistration { +export function createFactRetrieverRegistration({ + cadence, + factRetriever, + lifecycle, +}: FactRetrieverRegistrationOptions): FactRetrieverRegistration { return { cadence, factRetriever, diff --git a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts index b01bbe6248..0238721e99 100644 --- a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts +++ b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts @@ -16,7 +16,11 @@ import camelCase from 'lodash/camelCase'; import { Entity } from '@backstage/catalog-model'; import { get } from 'lodash'; -import { FactLifecycle, ITL, TTL } from '@backstage/plugin-tech-insights-node'; +import { + FactLifecycle, + MaxItems, + TTL, +} from '@backstage/plugin-tech-insights-node'; export const generateAnnotationFactName = (annotation: string) => camelCase(`hasAnnotation-${annotation}`); @@ -25,9 +29,9 @@ export const entityHasAnnotation = (entity: Entity, annotation: string) => Boolean(get(entity, ['metadata', 'annotations', annotation])); export const isTtl = (lifecycle: FactLifecycle): lifecycle is TTL => { - return !!(lifecycle as TTL).ttl; + return !!(lifecycle as TTL).timeToLive; }; -export const isItl = (lifecycle: FactLifecycle): lifecycle is ITL => { - return !!(lifecycle as ITL).itl; +export const isMaxItems = (lifecycle: FactLifecycle): lifecycle is MaxItems => { + return !!(lifecycle as MaxItems).maxItems; }; diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts index a1bb2dc299..07c01eb5b0 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -265,7 +265,7 @@ describe('Tech Insights database', () => { }); }); - it('should delete extraneous rows when ITL is defined. Should leave only n latest', async () => { + it('should delete extraneous rows when MaxItems is defined. Should leave only n latest', async () => { const deviledFact = (it: {}) => ({ ...it, facts: JSON.stringify({ @@ -289,11 +289,11 @@ describe('Tech Insights database', () => { testNumberFact: 555, }, }; - const itl = 2; - await store.insertFacts('test-fact', [factToBeInserted], { itl }); + const maxItems = 2; + await store.insertFacts('test-fact', [factToBeInserted], { maxItems }); const afterInsertionFacts = await testDbClient('facts').select(); - expect(afterInsertionFacts).toHaveLength(itl); + expect(afterInsertionFacts).toHaveLength(maxItems); expect(afterInsertionFacts[0]).toMatchObject( deviledFact(additionalFacts[0]), ); @@ -335,7 +335,7 @@ describe('Tech Insights database', () => { }, }; await store.insertFacts('test-fact', [factToBeInserted], { - ttl: { weeks: 2 }, + timeToLive: { weeks: 2 }, }); const afterInsertionFacts = await testDbClient('facts') diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index bb29657c07..682b7967bb 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -27,7 +27,7 @@ import { groupBy, omit } from 'lodash'; import { DateTime } from 'luxon'; import { Logger } from 'winston'; import { parseEntityName, stringifyEntityRef } from '@backstage/catalog-model'; -import { isItl, isTtl } from '../fact/factRetrievers/utils'; +import { isMaxItems, isTtl } from '../fact/factRetrievers/utils'; import Transaction = Knex.Transaction; export type RawDbFactRow = { @@ -108,12 +108,11 @@ export class TechInsightsDatabase implements TechInsightsStore { await tx.batchInsert('facts', factRows, this.CHUNK_SIZE); if (lifecycle && isTtl(lifecycle)) { - const expiration = DateTime.now().minus(lifecycle.ttl); + const expiration = DateTime.now().minus(lifecycle.timeToLive); await this.deleteExpiredFactsByDate(tx, factRows, expiration); } - if (lifecycle && isItl(lifecycle)) { - const items = lifecycle.itl; - await this.deleteExpiredFactsByNumber(tx, factRows, items); + if (lifecycle && isMaxItems(lifecycle)) { + await this.deleteExpiredFactsByNumber(tx, factRows, lifecycle.maxItems); } }); } @@ -204,7 +203,7 @@ export class TechInsightsDatabase implements TechInsightsStore { private async deleteExpiredFactsByNumber( tx: Transaction, factRows: { id: string; entity: string }[], - items: number, + maxItems: number, ) { const deletables = await tx('facts') .whereIn( @@ -220,7 +219,7 @@ export class TechInsightsDatabase implements TechInsightsStore { row_number() over (partition by id, entity order by timestamp desc) as fact_rank from facts) ranks where fact_rank <= ?? ) as filterjoin`, - items, + maxItems, ), joinClause => { joinClause diff --git a/plugins/tech-insights-node/src/facts.ts b/plugins/tech-insights-node/src/facts.ts index 34c753486a..7a70f92f92 100644 --- a/plugins/tech-insights-node/src/facts.ts +++ b/plugins/tech-insights-node/src/facts.ts @@ -196,29 +196,29 @@ export interface FactRetriever { * A Luxon duration like object for time to live value * * @example - * \{ ttl: 1209600000 \} - * \{ ttl: \{ weeks: 4 \} \} + * \{ timeToLive: 1209600000 \} + * \{ timeToLive: \{ weeks: 4 \} \} * **/ -export type TTL = { ttl: DurationLike }; +export type TTL = { timeToLive: DurationLike }; /** * @public * - * A number for items to live value + * A maximum number for items to be kept in the database for each fact retriever/entity pair * * @example - * \{ itl: 10 \} + * \{ maxItems: 10 \} * **/ -export type ITL = { itl: number }; +export type MaxItems = { maxItems: number }; /** * @public * * A fact lifecycle definition. Determines which strategy to use to purge expired facts from the database. */ -export type FactLifecycle = TTL | ITL; +export type FactLifecycle = TTL | MaxItems; /** * @public