-
Notifications
You must be signed in to change notification settings - Fork 301
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
feat: add copying files to/from container #676
base: main
Are you sure you want to change the base?
Changes from all commits
c08ae6c
8ed3df0
e7b7990
3040d2b
acd19f5
40fd110
a2f6094
fc40016
6f25274
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
import contextlib | ||
import io | ||
import os | ||
import tarfile | ||
from platform import system | ||
from socket import socket | ||
from typing import TYPE_CHECKING, Optional | ||
|
@@ -13,6 +16,7 @@ | |
from testcontainers.core.exceptions import ContainerStartException | ||
from testcontainers.core.labels import LABEL_SESSION_ID, SESSION_ID | ||
from testcontainers.core.network import Network | ||
from testcontainers.core.transferable import Transferable | ||
from testcontainers.core.utils import inside_container, is_arm, setup_logger | ||
from testcontainers.core.waiting_utils import wait_container_is_ready, wait_for_logs | ||
|
||
|
@@ -52,6 +56,7 @@ def __init__( | |
self._network: Optional[Network] = None | ||
self._network_aliases: Optional[list[str]] = None | ||
self._kwargs = kwargs | ||
self._files: list[Transferable] = [] | ||
|
||
def with_env(self, key: str, value: str) -> Self: | ||
self.env[key] = value | ||
|
@@ -78,6 +83,33 @@ def with_kwargs(self, **kwargs) -> Self: | |
self._kwargs = kwargs | ||
return self | ||
|
||
def with_copy_file_to_container(self, transferable: Transferable) -> Self: | ||
self._files.append(transferable) | ||
|
||
return self | ||
|
||
def copy_file_from_container(self, container_file: os.PathLike, destination_file: os.PathLike) -> os.PathLike: | ||
tar_stream, _ = self._container.get_archive(container_file) | ||
|
||
for chunk in tar_stream: | ||
with tarfile.open(fileobj=io.BytesIO(chunk)) as tar: | ||
for member in tar.getmembers(): | ||
with open(destination_file, "wb") as f: | ||
f.write(tar.extractfile(member).read()) | ||
|
||
return destination_file | ||
|
||
@staticmethod | ||
def _put_data_in_container(container, transferable: Transferable): | ||
data = io.BytesIO() | ||
|
||
with transferable as f, tarfile.open(fileobj=data, mode="w") as tar: | ||
tar.add(f.input_path, arcname=f.output_path) | ||
|
||
data.seek(0) | ||
|
||
container.put_archive("/", data) | ||
|
||
def maybe_emulate_amd64(self) -> Self: | ||
if is_arm(): | ||
return self.with_kwargs(platform="linux/amd64") | ||
|
@@ -115,6 +147,10 @@ def start(self) -> Self: | |
) | ||
|
||
logger.info("Container started: %s", self._container.short_id) | ||
|
||
for transferable in self._files: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found this PR noticing that copying files isn't supported, a bit surprising since it's a pretty important one and found in at least many of the other languages. I noticed this implementation doesn't seem to match others, though this is probably because of how the general docker running is implemented. Generally, the container is created, then files copied, then the container started, so the files are present when the command runs. Here, the files are just copied after startup so can't be used by the entrypoint - perhaps there's some use case for this but I don't know how common it is. Note that volume mounting is an alternative but doesn't always work due to e.g. permission mode issues - copying is generally more robust. Given the behavior would be so different from other implementations of testcontainers, I think this PR can cause a lot of confusion so probably needs to rework the lifecycle like that. |
||
DockerContainer._put_data_in_container(self._container, transferable) | ||
|
||
return self | ||
|
||
def stop(self, force=True, delete_volume=True) -> None: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import os | ||
import tempfile | ||
from typing import Union | ||
|
||
|
||
class Transferable: | ||
def __init__(self, input_data: Union[os.PathLike, bytes], output_path: os.PathLike): | ||
self._input = input_data | ||
self._output_path = output_path | ||
|
||
self._tmp_file: bool = False | ||
|
||
def __enter__(self): | ||
if isinstance(self._input, bytes): | ||
tmp_file = tempfile.NamedTemporaryFile(delete=False) | ||
tmp_file.write(self._input) | ||
|
||
self._input = tmp_file.name | ||
self._tmp_file = True | ||
|
||
return self | ||
|
||
def __exit__(self, *args): | ||
if self._tmp_file: | ||
os.remove(self._input) | ||
|
||
@property | ||
def input_path(self): | ||
return self._input | ||
|
||
@property | ||
def output_path(self): | ||
return self._output_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, the temporary file could be avoided if using
tar.addfile
- it is somewhat more tedious to set up aTarInfo
but maybe worth it to skip the temp fileThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think all the copy_file APIs in testcontainers accept a file mode. With the current pattern, it can be something like
filter=lambda tarinfo: tarinfo.mode = foo
, but if switching toaddfile
it could be set directly to theTarInfo