New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: sandbox client config options #643
Changes from 3 commits
4eb1063
4d691a4
ece9635
4cb5b72
70969c4
eebc71d
9589b49
79acaf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@aws-amplify/backend-cli': patch | ||
--- | ||
|
||
client config generation respects sandbox format option |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@aws-amplify/backend-cli': patch | ||
--- | ||
|
||
Rename sandbox format to config-format |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@aws-amplify/backend-cli': patch | ||
--- | ||
|
||
Change sandbox out-dir to config-out-dir and client config generation respects config-out-dir option |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,8 +19,8 @@ type SandboxCommandOptionsCamelCase = { | |
dirToWatch: string | undefined; | ||
exclude: string[] | undefined; | ||
name: string | undefined; | ||
format: ClientConfigFormat | undefined; | ||
outDir: string | undefined; | ||
configFormat: ClientConfigFormat | undefined; | ||
configOutDir: string | undefined; | ||
profile: string | undefined; | ||
}; | ||
|
||
|
@@ -81,7 +81,9 @@ export class SandboxCommand | |
|
||
// attaching event handlers | ||
const clientConfigLifecycleHandler = new ClientConfigLifecycleHandler( | ||
this.clientConfigGeneratorAdapter | ||
this.clientConfigGeneratorAdapter, | ||
args['config-out-dir'], | ||
args['config-format'] | ||
); | ||
const eventHandlers = this.sandboxEventHandlerCreator?.({ | ||
sandboxName: this.sandboxName, | ||
|
@@ -94,8 +96,8 @@ export class SandboxCommand | |
} | ||
const watchExclusions = args.exclude ?? []; | ||
const clientConfigWritePath = await getClientConfigPath( | ||
args['out-dir'], | ||
args.format | ||
args['config-out-dir'], | ||
args['config-format'] | ||
); | ||
watchExclusions.push(clientConfigWritePath); | ||
await sandbox.start({ | ||
|
@@ -136,14 +138,14 @@ export class SandboxCommand | |
array: false, | ||
global: false, | ||
}) | ||
.option('format', { | ||
.option('config-format', { | ||
describe: 'Client config output format', | ||
type: 'string', | ||
array: false, | ||
choices: Object.values(ClientConfigFormat), | ||
global: false, | ||
}) | ||
.option('out-dir', { | ||
.option('config-out-dir', { | ||
describe: | ||
'A path to directory where config is written. If not provided defaults to current process working directory.', | ||
type: 'string', | ||
|
@@ -157,20 +159,10 @@ export class SandboxCommand | |
}) | ||
.check((argv) => { | ||
if (argv['dir-to-watch']) { | ||
// make sure it's a real directory | ||
let stats; | ||
try { | ||
stats = fs.statSync(argv['dir-to-watch'], {}); | ||
} catch (e) { | ||
throw new Error( | ||
`--dir-to-watch ${argv['dir-to-watch']} does not exist` | ||
); | ||
} | ||
if (!stats.isDirectory()) { | ||
throw new Error( | ||
`--dir-to-watch ${argv['dir-to-watch']} is not a valid directory` | ||
); | ||
} | ||
this.validateDirectory('dir-to-watch', argv['dir-to-watch']); | ||
} | ||
if (argv['config-out-dir']) { | ||
this.validateDirectory('config-out-dir', argv['config-out-dir']); | ||
} | ||
if (argv.name) { | ||
const projectNameRegex = /^[a-zA-Z0-9-]{1,15}$/; | ||
|
@@ -201,4 +193,16 @@ export class SandboxCommand | |
await this.sandboxFactory.getInstance() | ||
).delete({ name: this.sandboxName }); | ||
}; | ||
|
||
validateDirectory = (option: string, dir: string) => { | ||
let stats; | ||
try { | ||
stats = fs.statSync(dir, {}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems that Ref: yargs/yargs#1872 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was not aware of this, good to know and done in 4cb5b72. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any file system access if possible should be made async so that event loop is not blocked. |
||
} catch (e) { | ||
throw new Error(`--${option} ${dir} does not exist`); | ||
} | ||
if (!stats.isDirectory()) { | ||
throw new Error(`--${option} ${dir} is not a valid directory`); | ||
} | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a test which asserts that these values are flowing correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 70969c4