-
Notifications
You must be signed in to change notification settings - Fork 4
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
Resolve bucket name dynamicly #3
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -28,6 +28,7 @@ export default class ServerlessS3Cleaner implements Plugin { | |
bucketsToCleanOnDeploy: { | ||
type: 'array', uniqueItems: true, items: { type: 'string' }, nullable: true | ||
}, | ||
autoResolve: { type: 'boolean', nullable: true, default: false }, | ||
}, | ||
additionalProperties: false, | ||
anyOf: [ | ||
|
@@ -67,6 +68,10 @@ export default class ServerlessS3Cleaner implements Plugin { | |
const config = this.loadConfig(); | ||
let bucketsToEmpty = isDeploying ? config.bucketsToCleanOnDeploy : config.buckets; | ||
|
||
if (!isDeploying && config.autoResolve) { | ||
bucketsToEmpty.push(await this.provider.getServerlessDeploymentBucketName()); | ||
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. Does this work properly with an explicit bucket name as well as a generated (default) bucket name? 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. In my mind logic is to delete deployment bucket only on 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. |
||
} | ||
|
||
if (config.prompt) { | ||
prompt.start(); | ||
const bucketPromptResults = await prompt.get(bucketsToEmpty.map(bucket => ({ | ||
|
@@ -177,6 +182,7 @@ export default class ServerlessS3Cleaner implements Plugin { | |
buckets: providedConfig.buckets || [], | ||
prompt: providedConfig.prompt || false, | ||
bucketsToCleanOnDeploy: providedConfig.bucketsToCleanOnDeploy || [], | ||
autoResolve: providedConfig.autoResolve || false, | ||
}; | ||
} | ||
} | ||
|
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.
The plugin is not meant to clean just the Serverless deployment buckets, think of stacks that create additional S3 buckets for other purposes, such as web apps. I think
autoResolve
in this context is a bit too open-ended, so I'd suggest a more targeted name. E.g.cleanDeploymentBucket
,includeDeploymentBucket
, or something similar?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.
@f00f sorry for mentioning ) just because you are appears in "maintainers"
@coyoteecd good point. For renaming option.