-
Notifications
You must be signed in to change notification settings - Fork 342
fix: get_wrapped_container returns container all the time now #906
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
base: main
Are you sure you want to change the base?
Conversation
ok, have to think about this one more carefully. seems like there is more to this than appears |
docker_compose_cmd += ["--env-file", env_file] | ||
return docker_compose_cmd | ||
|
||
def _get_docker_client(self) -> DockerClient: |
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.
def _get_docker_client(self) -> DockerClient: | |
def _get_docker_client(self) -> DockerClient: | |
if self._docker_client is None: | |
self._docker_client = DockerClient(**(self.docker_client_kw or {})) | |
return self._docker_client |
def get_wrapped_container(self) -> Container: | ||
"""Get the underlying container object for compatibility.""" | ||
return self | ||
return self._docker_compose._get_docker_client().containers.get(self.ID) |
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.
Dumb question: can .containers.get(self.ID)
return a value other than Container
, e.g. None?
what if the container is not found?
it throws docker api exception or something. its in the docstr of the get
method. just navigate to it and you should see it also
…On Thu, Oct 16, 2025 at 8:58 AM Ivan ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In core/testcontainers/compose/compose.py
<#906 (comment)>
:
> @@ -259,6 +263,13 @@ def compose_command_property(self) -> list[str]:
docker_compose_cmd += ["--env-file", env_file]
return docker_compose_cmd
+ def _get_docker_client(self) -> DockerClient:
⬇️ Suggested change
- def _get_docker_client(self) -> DockerClient:
+ def _get_docker_client(self) -> DockerClient:
+ if self._docker_client is None:
+ self._docker_client = DockerClient(**(self.docker_client_kw or {}))
+ return self._docker_client
------------------------------
In core/testcontainers/compose/compose.py
<#906 (comment)>
:
> """Get the underlying container object for compatibility."""
- return self
+ return self._docker_compose._get_docker_client().containers.get(self.ID)
Dumb question: can .containers.get(self.ID) return a value other than
Container, e.g. None?
what if the container is not found?
—
Reply to this email directly, view it on GitHub
<#906 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACECGJDPPFF3B2BECIZBBUD3X6I6RAVCNFSM6AAAAACJIPB3Z2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGNBUG4YTAMBUHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
com>
|
fix #905