Skip to content

Commit a15e73e

Browse files
authored
refactor(cli): settings doesn't log anymore (#267)
Removes logging from the `Settings` class. Logging was only used when loading settings from a file and that functionality is currently only used by the `UserConfiguration` class of the CLI. So I localized the loading there. If we wanted to re-introduce a generic loading from file in the future, we just need to build it with the `IoHost` in mind. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent f0d458c commit a15e73e

File tree

2 files changed

+62
-54
lines changed

2 files changed

+62
-54
lines changed

packages/aws-cdk/lib/api/settings.ts

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as os from 'os';
22
import * as fs_path from 'path';
33
import * as fs from 'fs-extra';
44
import { ToolkitError } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api';
5-
import { warning } from '../logging';
65
import * as util from '../util';
76

87
export type SettingsMap = { [key: string]: any };
@@ -30,27 +29,6 @@ export class Settings {
3029
) {
3130
}
3231

33-
public async load(fileName: string): Promise<this> {
34-
if (this.readOnly) {
35-
throw new ToolkitError(
36-
`Can't load ${fileName}: settings object is readonly`,
37-
);
38-
}
39-
this.settings = {};
40-
41-
const expanded = expandHomeDir(fileName);
42-
if (await fs.pathExists(expanded)) {
43-
this.settings = await fs.readJson(expanded);
44-
}
45-
46-
// See https://github.com/aws/aws-cdk/issues/59
47-
this.prohibitContextKey('default-account', fileName);
48-
this.prohibitContextKey('default-region', fileName);
49-
this.warnAboutContextKey('aws:', fileName);
50-
51-
return this;
52-
}
53-
5432
public async save(fileName: string): Promise<this> {
5533
const expanded = expandHomeDir(fileName);
5634
await fs.writeJson(expanded, stripTransientValues(this.settings), {
@@ -106,36 +84,6 @@ export class Settings {
10684
public unset(path: string[]) {
10785
this.set(path, undefined);
10886
}
109-
110-
private prohibitContextKey(key: string, fileName: string) {
111-
if (!this.settings.context) {
112-
return;
113-
}
114-
if (key in this.settings.context) {
115-
// eslint-disable-next-line max-len
116-
throw new ToolkitError(
117-
`The 'context.${key}' key was found in ${fs_path.resolve(
118-
fileName,
119-
)}, but it is no longer supported. Please remove it.`,
120-
);
121-
}
122-
}
123-
124-
private warnAboutContextKey(prefix: string, fileName: string) {
125-
if (!this.settings.context) {
126-
return;
127-
}
128-
for (const contextKey of Object.keys(this.settings.context)) {
129-
if (contextKey.startsWith(prefix)) {
130-
// eslint-disable-next-line max-len
131-
warning(
132-
`A reserved context key ('context.${prefix}') key was found in ${fs_path.resolve(
133-
fileName,
134-
)}, it might cause surprising behavior and should be removed.`,
135-
);
136-
}
137-
}
138-
}
13987
}
14088

14189
function expandHomeDir(x: string) {

packages/aws-cdk/lib/cli/user-configuration.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import * as os from 'os';
2+
import * as fs_path from 'path';
3+
import * as fs from 'fs-extra';
14
import { ToolkitError } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api';
25
import { Context, PROJECT_CONTEXT } from '../api/context';
36
import { Settings } from '../api/settings';
@@ -174,14 +177,71 @@ export class Configuration {
174177
}
175178

176179
async function loadAndLog(fileName: string): Promise<Settings> {
177-
const ret = new Settings();
178-
await ret.load(fileName);
180+
const ret = await settingsFromFile(fileName);
179181
if (!ret.empty) {
180182
debug(fileName + ':', JSON.stringify(ret.all, undefined, 2));
181183
}
182184
return ret;
183185
}
184186

187+
async function settingsFromFile(fileName: string): Promise<Settings> {
188+
let settings;
189+
const expanded = expandHomeDir(fileName);
190+
if (await fs.pathExists(expanded)) {
191+
const data = await fs.readJson(expanded);
192+
settings = new Settings(data);
193+
} else {
194+
settings = new Settings();
195+
}
196+
197+
// See https://github.com/aws/aws-cdk/issues/59
198+
prohibitContextKeys(settings, ['default-account', 'default-region'], fileName);
199+
warnAboutContextKey(settings, 'aws:', fileName);
200+
201+
return settings;
202+
}
203+
204+
function prohibitContextKeys(settings: Settings, keys: string[], fileName: string) {
205+
const context = settings.get(['context']);
206+
if (!context || typeof context !== 'object') {
207+
return;
208+
}
209+
210+
for (const key of keys) {
211+
if (key in context) {
212+
throw new ToolkitError(
213+
`The 'context.${key}' key was found in ${fs_path.resolve(
214+
fileName,
215+
)}, but it is no longer supported. Please remove it.`,
216+
);
217+
}
218+
}
219+
}
220+
221+
function warnAboutContextKey(settings: Settings, prefix: string, fileName: string) {
222+
const context = settings.get(['context']);
223+
if (!context || typeof context !== 'object') {
224+
return;
225+
}
226+
227+
for (const contextKey of Object.keys(context)) {
228+
if (contextKey.startsWith(prefix)) {
229+
warning(
230+
`A reserved context key ('context.${prefix}') key was found in ${fs_path.resolve(
231+
fileName,
232+
)}, it might cause surprising behavior and should be removed.`,
233+
);
234+
}
235+
}
236+
}
237+
238+
function expandHomeDir(x: string) {
239+
if (x.startsWith('~')) {
240+
return fs_path.join(os.homedir(), x.slice(1));
241+
}
242+
return x;
243+
}
244+
185245
/**
186246
* Parse CLI arguments into Settings
187247
*

0 commit comments

Comments
 (0)