Skip to content

Commit

Permalink
Merge branch 'master' into sfn-tasks-aws-sdk-integ-pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Feb 28, 2022
2 parents 0e2d97b + 5c40b90 commit 587e7ea
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 73 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class Bundling implements cdk.BundlingOptions {
private readonly packageManager: PackageManager;

constructor(private readonly props: BundlingProps) {
this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath);
this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath, props.logLevel);

Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? PackageInstallation.detect('esbuild');
Bundling.tscInstallation = Bundling.tscInstallation ?? PackageInstallation.detect('tsc');
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as path from 'path';
import * as lambda from '@aws-cdk/aws-lambda';
import { Architecture } from '@aws-cdk/aws-lambda';
import { Bundling } from './bundling';
import { PackageManager } from './package-manager';
import { LockFile } from './package-manager';
import { BundlingOptions } from './types';
import { callsites, findUpMultiple } from './util';

Expand Down Expand Up @@ -138,9 +138,9 @@ function findLockFile(depsLockFilePath?: string): string {
}

const lockFiles = findUpMultiple([
PackageManager.PNPM.lockFile,
PackageManager.YARN.lockFile,
PackageManager.NPM.lockFile,
LockFile.PNPM,
LockFile.YARN,
LockFile.NPM,
]);

if (lockFiles.length === 0) {
Expand Down
62 changes: 34 additions & 28 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,54 @@
import * as os from 'os';
import * as path from 'path';
import { LogLevel } from './types';

interface PackageManagerProps {
readonly lockFile: string;
readonly installCommand: string[];
readonly runCommand: string[];
readonly argsSeparator?: string
readonly argsSeparator?: string;
}

export enum LockFile {
NPM = 'package-lock.json',
YARN = 'yarn.lock',
PNPM = 'pnpm-lock.yaml',
}

/**
* A node package manager
*/
export class PackageManager {
public static NPM = new PackageManager({
lockFile: 'package-lock.json',
installCommand: ['npm', 'ci'],
runCommand: ['npx', '--no-install'],
});

public static YARN = new PackageManager({
lockFile: 'yarn.lock',
installCommand: ['yarn', 'install', '--no-immutable'],
runCommand: ['yarn', 'run'],
});

public static PNPM = new PackageManager({
lockFile: 'pnpm-lock.yaml',
installCommand: ['pnpm', 'install'],
runCommand: ['pnpm', 'exec'],
argsSeparator: '--',
});

public static fromLockFile(lockFilePath: string): PackageManager {
/**
* Use a lock file path to determine the package manager to use. Optionally, specify a log level to
* control its verbosity.
* @param lockFilePath Path of the lock file
* @param logLevel optional log level @default LogLevel.INFO
* @returns the right PackageManager for that lock file
*/
public static fromLockFile(lockFilePath: string, logLevel?: LogLevel): PackageManager {
const lockFile = path.basename(lockFilePath);

switch (lockFile) {
case PackageManager.NPM.lockFile:
return PackageManager.NPM;
case PackageManager.YARN.lockFile:
return PackageManager.YARN;
case PackageManager.PNPM.lockFile:
return PackageManager.PNPM;
case LockFile.YARN:
return new PackageManager({
lockFile: LockFile.YARN,
installCommand: logLevel && logLevel !== LogLevel.INFO ? ['yarn', 'install', '--no-immutable', '--silent'] : ['yarn', 'install', '--no-immutable'],
runCommand: ['yarn', 'run'],
});
case LockFile.PNPM:
return new PackageManager({
lockFile: LockFile.PNPM,
installCommand: logLevel && logLevel !== LogLevel.INFO ? ['pnpm', 'install', '--reporter', 'silent'] : ['pnpm', 'install'],
runCommand: ['pnpm', 'exec'],
argsSeparator: '--',
});
default:
return PackageManager.NPM;
return new PackageManager({
lockFile: LockFile.NPM,
installCommand: logLevel ? ['npm', 'ci', '--loglevel', logLevel] : ['npm', 'ci'],
runCommand: ['npx', '--no-install'],
});
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ export interface BundlingOptions {
readonly loader?: { [ext: string]: string };

/**
* Log level for esbuild
* Log level for esbuild. This is also propagated to the package manager and
* applies to its specific install command.
*
* @default LogLevel.WARNING
*/
Expand Down Expand Up @@ -340,7 +341,7 @@ export interface ICommandHooks {
}

/**
* Log level for esbuild
* Log levels for esbuild and package managers' install commands.
*/
export enum LogLevel {
/** Show everything */
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ test('Local bundling', () => {
environment: {
KEY: 'value',
},
logLevel: LogLevel.ERROR,
});

expect(bundler.local).toBeDefined();
Expand Down
38 changes: 32 additions & 6 deletions packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,62 @@
import * as os from 'os';
import { PackageManager } from '../lib/package-manager';
import { LogLevel } from '../lib';
import { LockFile, PackageManager } from '../lib/package-manager';

test('from a package-lock.json', () => {
const packageManager = PackageManager.fromLockFile('/path/to/package-lock.json');
expect(packageManager).toEqual(PackageManager.NPM);
expect(packageManager.lockFile).toEqual(LockFile.NPM);
expect(packageManager.argsSeparator).toBeUndefined();
expect(packageManager.installCommand).toEqual(['npm', 'ci']);
expect(packageManager.runCommand).toEqual(['npx', '--no-install']);

expect(packageManager.runBinCommand('my-bin')).toBe('npx --no-install my-bin');
});

test('from a package-lock.json with LogLevel.ERROR', () => {
const logLevel = LogLevel.ERROR;
const packageManager = PackageManager.fromLockFile('/path/to/package-lock.json', logLevel);
expect(packageManager.installCommand).toEqual(['npm', 'ci', '--loglevel', logLevel]);
});

test('from a yarn.lock', () => {
const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock');
expect(packageManager).toEqual(PackageManager.YARN);
expect(packageManager.lockFile).toEqual(LockFile.YARN);
expect(packageManager.argsSeparator).toBeUndefined();
expect(packageManager.installCommand).toEqual(['yarn', 'install', '--no-immutable']);
expect(packageManager.runCommand).toEqual(['yarn', 'run']);

expect(packageManager.runBinCommand('my-bin')).toBe('yarn run my-bin');
});

test('from a yarn.lock with LogLevel.ERROR', () => {
const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock', LogLevel.ERROR);
expect(packageManager.installCommand).toEqual(['yarn', 'install', '--no-immutable', '--silent']);
});

test('from a pnpm-lock.yaml', () => {
const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml');
expect(packageManager).toEqual(PackageManager.PNPM);
expect(packageManager.lockFile).toEqual(LockFile.PNPM);
expect(packageManager.argsSeparator).toEqual('--');
expect(packageManager.installCommand).toEqual(['pnpm', 'install']);
expect(packageManager.runCommand).toEqual(['pnpm', 'exec']);

expect(packageManager.runBinCommand('my-bin')).toBe('pnpm exec -- my-bin');
});

test('from a pnpm-lock.yaml with LogLevel.ERROR', () => {
const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml', LogLevel.ERROR);
expect(packageManager.installCommand).toEqual(['pnpm', 'install', '--reporter', 'silent']);
});

test('defaults to NPM', () => {
const packageManager = PackageManager.fromLockFile('/path/to/other.lock');
expect(packageManager).toEqual(PackageManager.NPM);
expect(packageManager.lockFile).toEqual(LockFile.NPM);
});

test('Windows', () => {
const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32');

const packageManager = PackageManager.NPM;
const packageManager = PackageManager.fromLockFile('/path/to/whatever');
expect(packageManager.runBinCommand('my-bin')).toEqual('npx.cmd --no-install my-bin');

osPlatformMock.mockRestore();
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-lambda-python/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,13 @@ export class Bundling implements CdkBundlingOptions {
outputDir: outputPath,
});

const defaultImage = DockerImage.fromBuild(path.join(__dirname, '../lib'), {
this.image = image ?? DockerImage.fromBuild(path.join(__dirname, '../lib'), {
buildArgs: {
...props.buildArgs ?? {},
IMAGE: runtime.bundlingImage.image,
},
platform: architecture.dockerPlatform,
});
this.image = image ?? defaultImage;
this.command = ['bash', '-c', chain(bundlingCommands)];
this.environment = props.environment;
}
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-lambda-python/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ test('Bundling a function with custom bundling image', () => {
}),
}));

expect(DockerImage.fromBuild).toHaveBeenCalledTimes(1);
expect(DockerImage.fromBuild).toHaveBeenCalledWith(expect.stringMatching(entry));
});

Expand Down
69 changes: 41 additions & 28 deletions packages/aws-cdk/lib/notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,37 +105,46 @@ export interface NoticeDataSource {

export class WebsiteNoticeDataSource implements NoticeDataSource {
fetch(): Promise<Notice[]> {
const timeout = 3000;

return new Promise((resolve) => {
try {
const req = https.get('https://cli.cdk.dev-tools.aws.dev/notices.json', res => {
if (res.statusCode === 200) {
res.setEncoding('utf8');
let rawData = '';
res.on('data', (chunk) => {
rawData += chunk;
});
res.on('end', () => {
try {
const data = JSON.parse(rawData).notices as Notice[];
resolve(data ?? []);
} catch (e) {
debug(`Failed to parse notices: ${e}`);
const req = https.get('https://cli.cdk.dev-tools.aws.dev/notices.json',
{ timeout },
res => {
const startTime = Date.now();
if (res.statusCode === 200) {
res.setEncoding('utf8');
let rawData = '';
res.on('data', (chunk) => {
if (Date.now() - startTime > timeout) {
resolve([]);
}
rawData += chunk;
});
res.on('end', () => {
try {
const data = JSON.parse(rawData).notices as Notice[];
resolve(data ?? []);
} catch (e) {
debug(`Failed to parse notices: ${e}`);
resolve([]);
}
});
res.on('error', e => {
debug(`Failed to fetch notices: ${e}`);
resolve([]);
}
});
res.on('error', e => {
debug(`Failed to fetch notices: ${e}`);
});
} else {
debug(`Failed to fetch notices. Status code: ${res.statusCode}`);
resolve([]);
});
} else {
debug(`Failed to fetch notices. Status code: ${res.statusCode}`);
resolve([]);
}
});
}
});
req.on('error', e => {
debug(`Error on request: ${e}`);
resolve([]);
});
req.on('timeout', _ => resolve([]));
} catch (e) {
debug(`HTTPS 'get' call threw an error: ${e}`);
resolve([]);
Expand Down Expand Up @@ -176,14 +185,18 @@ export class CachedDataSource implements NoticeDataSource {
}

private async load(): Promise<CachedNotices> {
const defaultValue = {
expiration: 0,
notices: [],
};

try {
return await fs.readJSON(this.fileName) as CachedNotices;
return fs.existsSync(this.fileName)
? await fs.readJSON(this.fileName) as CachedNotices
: defaultValue;
} catch (e) {
debug(`Failed to load notices from cache: ${e}`);
return {
expiration: 0,
notices: [],
};
return defaultValue;
}
}

Expand Down
35 changes: 33 additions & 2 deletions packages/aws-cdk/test/notices.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as os from 'os';
import * as path from 'path';
import * as fs from 'fs-extra';
import * as nock from 'nock';
import * as logging from '../lib/logging';
import {
CachedDataSource,
filterNotices,
Expand Down Expand Up @@ -233,6 +234,32 @@ describe('cli notices', () => {
expect(result).toEqual([]);
});

test('returns empty array when the connection stays idle for too long', async () => {
nock('https://cli.cdk.dev-tools.aws.dev')
.get('/notices.json')
.delayConnection(3500)
.reply(200, {
notices: [BASIC_NOTICE],
});

const result = await dataSource.fetch();

expect(result).toEqual([]);
});

test('returns empty array when the request takes too long to finish', async () => {
nock('https://cli.cdk.dev-tools.aws.dev')
.get('/notices.json')
.delayBody(3500)
.reply(200, {
notices: [BASIC_NOTICE],
});

const result = await dataSource.fetch();

expect(result).toEqual([]);
});

function mockCall(statusCode: number, body: any): Promise<Notice[]> {
nock('https://cli.cdk.dev-tools.aws.dev')
.get('/notices.json')
Expand Down Expand Up @@ -284,12 +311,16 @@ describe('cli notices', () => {
});

test('retrieves data from the delegate when the file cannot be read', async () => {
const nonExistingFile = path.join(os.tmpdir(), 'cache.json');
const dataSource = dataSourceWithDelegateReturning(freshData, nonExistingFile);
const debugSpy = jest.spyOn(logging, 'debug');

const dataSource = dataSourceWithDelegateReturning(freshData, 'does-not-exist.json');

const notices = await dataSource.fetch();

expect(notices).toEqual(freshData);
expect(debugSpy).not.toHaveBeenCalled();

debugSpy.mockRestore();
});

test('retrieved data from the delegate when it is configured to ignore the cache', async () => {
Expand Down

0 comments on commit 587e7ea

Please sign in to comment.