Skip to content
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

Using KeyPrefix does not delete the s3 bucket after "sls remove" command. #134

Closed
1 task done
thescientist23 opened this issue Jan 18, 2022 · 6 comments · Fixed by #158
Closed
1 task done

Using KeyPrefix does not delete the s3 bucket after "sls remove" command. #134

thescientist23 opened this issue Jan 18, 2022 · 6 comments · Fixed by #158
Assignees
Labels

Comments

@thescientist23
Copy link

thescientist23 commented Jan 18, 2022

This is a:

  • [] Feature request or change
  • Bug report

For bug reports:

Expected behavior

"sls remove" should remove the bucket

Actual behavior

objects was deleted but the bucket remains, although it is empty but in a long run it will pile up on my aws account.

Steps to reproduce

Added KeyPrefix to yml file.

custom:
   client:
     keyPrefix: 'samplepref'

Below is the result of "remove" command.
Screen Shot 2022-01-18 at 3 33 34 PM

I think this block of code has something to do with the behavior.
Screen Shot 2022-01-18 at 3 38 11 PM

  • **Operating system:osx
  • **serverless-finch version:2.8.0
  • serverless.yml that produces bug:
  • Command that produces bug:
  • Other details:

For feature requests or changes:

Current behavior (if any)

Proposed behavior

Proposed implementation deatils (optional)

Justification

@mikejpeters
Copy link
Collaborator

On one hand I think it makes sense not to remove the bucket when keyPrefix is being used, since ait could be used used when deploying to a bucket that also contains other data. But there may be other use cases, such as making the deployment match the naming conventions of frontend frameworks / tools.

@thescientist23 if you're still interested in this functionality can you please explain why you're using keyPrefix but also want the bucket removed?

I think if we want to support removing the bucket when keyPrefix is in use it should be using a new option.

@fernando-mc
Copy link
Owner

@mikejpeters seconding what you're saying here. This could have unintended consequences that I'm not comfortable with. If we add support for this it should be in a new feature/setting.

@thescientist23
Copy link
Author

@mikejpeters , thanks for your response, I am using KeyPrefix for my custom domain, basically for developing feature branch, eg. https://domain.com/feat-1, https://domain.com/feat-2,...etc. and to map the location according to feature branch I am deploying the react app on non root path thats where the KeyPrefix came into play.

@mikejpeters
Copy link
Collaborator

Thanks for the response. I'm assuming you wouldn't want this plugin to delete the bucket if there are still other objects i (e.g. in your case feature branches)?

@fernando-mc can you weigh in on this again when you have a chance? If the plugin deletes the objects at the prefix, and then only deletes the bucket if it's completely empty I don't see a downside. But maybe I'm missing some danger / use case? Having a new option like "force remove bucket" is safer but IMO less user-friendly. I like having smart defaults / less options, even if it means releasing a new major version with breaking changes

@fernando-mc
Copy link
Owner

@mikejpeters - I also prefer clean sensible defaults so as long as we release this with a major version bump I'm fine with it. I agree it probably makes sense and there's aren't many people who would object to it but it does change default behavior destructively and there's always some weird use cases we can't predict.

When including this:

  1. Add a warning to release notes
  2. Bump a major version

Then we're covering our bases and should be fine.

@mikejpeters mikejpeters self-assigned this Feb 28, 2022
mikejpeters added a commit that referenced this issue Mar 2, 2022
When running `client remove`, if a bucket only contains objects at the keyPrefix then also delete
the bucket itself.

BREAKING CHANGE: Bucket is deleted if empty on `client remove`.

resolves #134
mikejpeters added a commit that referenced this issue Mar 3, 2022
When running `client remove`, if a bucket only contains objects at the keyPrefix then also delete
the bucket itself.

BREAKING CHANGE: Bucket is deleted if empty on `client remove`.

resolves #134
@github-actions
Copy link

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants