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

fixed removal of cloudfront distribution #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

evgsil
Copy link
Contributor

@evgsil evgsil commented Sep 25, 2019

Previously active (enabled) distributions were never removed only disabled

@eahefnawy
Copy link
Member

eahefnawy commented Sep 26, 2019

@evgsil wish you opened up an issue first. Wouldn't that cause the removal to take a long time to complete? Which would affect overall deployment. This is something that happens with CloudFormation that we wanted to avoid both for deployment and removal.

@evgsil
Copy link
Contributor Author

evgsil commented Sep 26, 2019

Yes, it would take much more time, but it actually does the job (distribution is removed). As far as i understand, slow speed is not CloudFormation issue but issue of CloudFront itself. It just takes a lot of time for AWS to sync changes between CloudFront edge locations.

@eahefnawy
Copy link
Member

@evgsil I'm more concerned with the DX of this. Based on my experience with both CloudFormation and components, I deploy and remove quite often during development. Having to wait this long every time is killer. Imo it's just easier to manually remove the distribution if you really MUST do that. Alternativley, we might have that as an input option (remove: true).

This is even more problematic that we don't yet have remote deployment engine, so your terminal must be running this whole time for the deployment/removal to complete. Maybe once we have remote deployment engine, it might make more sense.

I'm gonna leave this PR open for more feedback. Hope you understand 😊

@urtens
Copy link

urtens commented Jul 1, 2020

Personally I wouldn't mind waiting for the distribution to be actually deleted, specially if that's what I'm expecting. Users are not warned about it.
An input option sounds like a good compromise.

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

Successfully merging this pull request may close these issues.

3 participants