Skip to content

Commit

Permalink
Merge branch 'master' into ai-assistant-node-data-improvements
Browse files Browse the repository at this point in the history
* master:
  refactor(core): Have `WorkerServer` use `InstanceSettings` (#10840)
  • Loading branch information
MiloradFilipovic committed Sep 17, 2024
2 parents 8aa94fc + 7f4ef31 commit 7173ef0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 13 deletions.
36 changes: 25 additions & 11 deletions packages/cli/src/scaling/__tests__/worker-server.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { GlobalConfig } from '@n8n/config';
import type express from 'express';
import { mock } from 'jest-mock-extended';
import type { InstanceSettings } from 'n8n-core';
import { AssertionError } from 'node:assert';
import * as http from 'node:http';

import config from '@/config';
import { PortTakenError } from '@/errors/port-taken.error';
import type { ExternalHooks } from '@/external-hooks';
import { bodyParser, rawBodyReader } from '@/middlewares';
Expand All @@ -27,9 +27,9 @@ describe('WorkerServer', () => {
let globalConfig: GlobalConfig;

const externalHooks = mock<ExternalHooks>();
const instanceSettings = mock<InstanceSettings>({ instanceType: 'worker' });

beforeEach(() => {
config.set('generic.instanceType', 'worker');
globalConfig = mock<GlobalConfig>({
queue: {
health: { active: true, port: 5678 },
Expand All @@ -43,10 +43,16 @@ describe('WorkerServer', () => {

describe('constructor', () => {
it('should throw if non-worker instance type', () => {
config.set('generic.instanceType', 'webhook');

expect(
() => new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks),
() =>
new WorkerServer(
globalConfig,
mock(),
mock(),
mock(),
externalHooks,
mock<InstanceSettings>({ instanceType: 'webhook' }),
),
).toThrowError(AssertionError);
});

Expand All @@ -61,14 +67,15 @@ describe('WorkerServer', () => {
});

expect(
() => new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks),
() =>
new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks, instanceSettings),
).toThrowError(PortTakenError);
});

it('should set up `/healthz` if health check is enabled', async () => {
jest.spyOn(http, 'createServer').mockReturnValue(mock<http.Server>());

new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks);
new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks, instanceSettings);

expect(app.get).toHaveBeenCalledWith('/healthz', expect.any(Function));
});
Expand All @@ -78,7 +85,7 @@ describe('WorkerServer', () => {

jest.spyOn(http, 'createServer').mockReturnValue(mock<http.Server>());

new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks);
new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks, instanceSettings);

expect(app.get).not.toHaveBeenCalled();
});
Expand All @@ -89,7 +96,7 @@ describe('WorkerServer', () => {
const CREDENTIALS_OVERWRITE_ENDPOINT = 'credentials/overwrites';
globalConfig.credentials.overwrite.endpoint = CREDENTIALS_OVERWRITE_ENDPOINT;

new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks);
new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks, instanceSettings);

expect(app.post).toHaveBeenCalledWith(
`/${CREDENTIALS_OVERWRITE_ENDPOINT}`,
Expand All @@ -102,7 +109,7 @@ describe('WorkerServer', () => {
it('should not set up `/:endpoint` if overwrites endpoint is not set', async () => {
jest.spyOn(http, 'createServer').mockReturnValue(mock<http.Server>());

new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks);
new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks, instanceSettings);

expect(app.post).not.toHaveBeenCalled();
});
Expand All @@ -118,7 +125,14 @@ describe('WorkerServer', () => {
return server;
});

const workerServer = new WorkerServer(globalConfig, mock(), mock(), mock(), externalHooks);
const workerServer = new WorkerServer(
globalConfig,
mock(),
mock(),
mock(),
externalHooks,
instanceSettings,
);
await workerServer.init();

expect(externalHooks.run).toHaveBeenCalledWith('worker.ready');
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/scaling/worker-server.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { GlobalConfig } from '@n8n/config';
import express from 'express';
import { InstanceSettings } from 'n8n-core';
import { ensureError } from 'n8n-workflow';
import { strict as assert } from 'node:assert';
import http from 'node:http';
import type { Server } from 'node:http';
import { Service } from 'typedi';

import config from '@/config';
import { CredentialsOverwrites } from '@/credentials-overwrites';
import * as Db from '@/db';
import { CredentialsOverwritesAlreadySetError } from '@/errors/credentials-overwrites-already-set.error';
Expand Down Expand Up @@ -40,8 +40,9 @@ export class WorkerServer {
private readonly scalingService: ScalingService,
private readonly credentialsOverwrites: CredentialsOverwrites,
private readonly externalHooks: ExternalHooks,
private readonly instanceSettings: InstanceSettings,
) {
assert(config.getEnv('generic.instanceType') === 'worker');
assert(this.instanceSettings.instanceType === 'worker');

const app = express();

Expand Down

0 comments on commit 7173ef0

Please sign in to comment.