Skip to content

fix: get_wrapped_container returns container all the time now#906

Open
alexanderankin wants to merge 1 commit intomainfrom
fix/less-crazy-get_wrapped_container
Open

fix: get_wrapped_container returns container all the time now#906
alexanderankin wants to merge 1 commit intomainfrom
fix/less-crazy-get_wrapped_container

Conversation

@alexanderankin
Copy link
Copy Markdown
Member

fix #905

@alexanderankin
Copy link
Copy Markdown
Member Author

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:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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)
Copy link
Copy Markdown

@surister surister Oct 16, 2025

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?

@alexanderankin
Copy link
Copy Markdown
Member Author

alexanderankin commented Oct 16, 2025 via email

...

def get_wrapped_container(self) -> Any:
def get_wrapped_container(self) -> Container:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We recently had a PR that added a bunch of data classes, I think I should actually be returning one of those here

@Tranquility2 Tranquility2 added 🔧 fix 🛠️ needs more work Need to invest more time, can be a rebase or code updates labels Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔧 fix 🛠️ needs more work Need to invest more time, can be a rebase or code updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: HealthcheckWaitStrategy returns error with DockerCompose

4 participants