Allow for optionally passing options to #remove
#31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HeyNonster
force-pushed
the
nony--pass-in-options-to-remove
branch
from
65601e8 to
1be8365
Compare
HeyNonster
changed the title
.remove#remove
|
@HeyNonster hi! can you run |
The `Docker::Container` `remove` method in [docker-api conditionally accepts a hash of options](https://github.com/upserve/docker-api/blob/6f7b7cd2b790ec0ca69f805d72ae0c504c8b2f49/lib/docker/container.rb#L248) Furthermore, removing a docker container, by default, will [not remove the volumes](https://docs.docker.com/engine/api/v1.43/#tag/Container/operation/ContainerDelete) unless you pass `v: true` as a parameter. Without the ability to pass `v: true` we can end up with many dangling volumes that take up disk space. This commit adds an optional hash argument to the `DockerContainer` `#remove` method, it defaults to an empty hash. To assist with tests I've also added: - `DockerContainer#mounts` method which returns an array of the mount hashes from the container `#info` - `DockerContainer#mount_names` which returns an array of the mount names (ids)
HeyNonster
force-pushed
the
nony--pass-in-options-to-remove
branch
from
1be8365 to
bb66e6d
Compare
@guilleiguaran Done! :) |
|
Seems to all be passing except for the |
|
@HeyNonster thanks!!! I'll fix the timeout issue later this week (or feel free to send a PR for it if you have some spare time :D) |
@guilleiguaran You probably already saw this, but: #32 🙂 |
HeyNonster
added a commit
to HeyNonster/testcontainers-ruby
that referenced
this pull request
Dec 14, 2023
The [Docker ImageCreate](https://docs.docker.com/engine/api/v1.43/#tag/Image/operation/ImageCreate) api accepts a variety of parameters. For the `platform` behavior, there is default behavior which is notable: ``` Default: "" When used in combination with the fromImage option, the daemon checks if the given image is present in the local image cache with the given OS and Architecture, and otherwise attempts to pull the image. If the option is not set, the host's native OS and Architecture are used. ... ``` Because it defauilts to the host's native architecture, it will attempt to pull images matching that architecture and raises not found if an image of that architecture doesn't exist. This can be a problem when, for example, attempting to pull a `mysql` image on Mx Macs. `Testcontainers::DockerContainer.create('mysql:5.7')` will fail because, as of this writing, arm images are not created for `mysql:5.7`. The solution to this is to pass `platform: 'linux/amd64'` to `Docker::Image.create`. This tells Docker we specifically want a image for that architecture (Rosetta on Mac OS X can run `linux/amd64` images). This commit adds an optional `image_create_options` kwarg to `DockerContainer#initialize` that defaults to an empty hash. This kwarg allows for us to pass create parameters to `Docker::Image.create`. I've also added a `NotFoundError` class because it seems that the convention is to wrap dependency errors in a `Textcontainers` error class. Updates the readme, including a readme addition to explain the changes in testcontainers#31.
HeyNonster
added a commit
to HeyNonster/testcontainers-ruby
that referenced
this pull request
Dec 14, 2023
The [Docker ImageCreate](https://docs.docker.com/engine/api/v1.43/#tag/Image/operation/ImageCreate) api accepts a variety of parameters. For the `platform` behavior, there is default behavior which is notable: ``` Default: "" When used in combination with the fromImage option, the daemon checks if the given image is present in the local image cache with the given OS and Architecture, and otherwise attempts to pull the image. If the option is not set, the host's native OS and Architecture are used. ... ``` Because it defauilts to the host's native architecture, it will attempt to pull images matching that architecture and raises not found if an image of that architecture doesn't exist. This can be a problem when, for example, attempting to pull a `mysql` image on Mx Macs. `Testcontainers::DockerContainer.create('mysql:5.7')` will fail because, as of this writing, arm images are not created for `mysql:5.7`. The solution to this is to pass `platform: 'linux/amd64'` to `Docker::Image.create`. This tells Docker we specifically want a image for that architecture (Rosetta on Mac OS X can run `linux/amd64` images). This commit adds an optional `image_create_options` kwarg to `DockerContainer#initialize` that defaults to an empty hash. This kwarg allows for us to pass create parameters to `Docker::Image.create`. I've also added a `NotFoundError` class because it seems that the convention is to wrap dependency errors in a `Textcontainers` error class. Updates the readme, including a readme addition to explain the changes in testcontainers#31.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Docker::Containerremovemethod in docker-api conditionally accepts a hash of optionsFurthermore, removing a docker container, by default, will not remove the volumes unless you pass
v: trueas a parameter.Without the ability to pass
v: truewe can end up with many dangling volumes that take up disk space.This commit adds an optional hash argument to the
DockerContainer#removemethod, it defaults to an empty hash.To assist with tests I've also added:
DockerContainer#mountsmethod which returns an array of the mount hashes from the container#infoDockerContainer#mount_nameswhich returns an array of the mount names (ids)Not changed but perhaps good to discuss: should
removealso remove the volumes by default?