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

Rename container when recreating it #1349

Merged
merged 1 commit into from
May 8, 2015

Conversation

dnephin
Copy link

@dnephin dnephin commented Apr 24, 2015

Resolves #874 (and a bunch of related bug reports)


options = dict(override_options)
# Use a hopefully unique container name by prepending the short id
self.client.rename(container.id, container.short_id + container.name)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is one possible failure case here. If compose crashes between this line , and the remove() line, the old container will be left with the temporary name.

The next time compose runs, it wont recreate (since the container has a different name now), and any volumes will be new instances. The old volumes will still be attached to the temporary container, so they shouldn't be lost.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also possible right now, isn't it? If Compose crashes after creating the intermediate container but before creating the new one, it won't recreate.

It's not ideal, but at least it isn't a regression ^_^

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny thing, but right now this generates names like c8a6039dcecounter_web_1. Could we stick an underscore between the short ID and the name?

@aanand
Copy link

aanand commented May 1, 2015

Should close #919 too.

@dnephin dnephin force-pushed the rename_instead_of_intermediate branch from 067dd8b to 6829efd Compare May 8, 2015 01:53
@dnephin
Copy link
Author

dnephin commented May 8, 2015

Rebased, and added the underscore to the renamed container name

@aanand
Copy link

aanand commented May 8, 2015

giphy

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.

Drop intermediate container and use renaming instead
3 participants