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

Delegate copyFile{To,From}Container's state verification to Docker #3805

Merged

Conversation

michael-simons
Copy link
Contributor

This PR refers to #3555 and #3266 and takes the work of @eastlondoner and the recommendations of @bsideup into consideration. The main takeaways are:

  • Use the presence of a container id to check whether an attempt to copy a file should be made or not (As suggested from Sergei "Alternatively, the check can be relaxed to getContainerId() != null and then the Docker API call would fail.")
  • Don't change the behavior of isCreated
  • Remove the additional companion object for ContainerState and it's exists method
  • Polish the tests (remove the folder checks, the busy loops for the container checks where possible) and try to create some order to mirror the possible states of a container that allows copying files

@bsideup bsideup added this to the next milestone Feb 15, 2021
@bsideup bsideup changed the title Just check for the existence of a container before copying to and from it. Delegate copyFile{To,From}Container's state verification to Docker Feb 15, 2021
@bsideup bsideup merged commit eaf9b9f into testcontainers:master Feb 15, 2021
@bsideup
Copy link
Member

bsideup commented Feb 15, 2021

Merged! Thank you @eastlondoner and @michael-simons for working on it! 🎉

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

Successfully merging this pull request may close these issues.

None yet

2 participants