8000 fix: no connect to docker on startup by alexanderankin · Pull Request #831 · testcontainers/testcontainers-python · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@alexanderankin
Copy link
Member

fix #830

@CarliJoy
Copy link
Contributor

Could we get rid of all UPPER_CASE variables?
They were kept for backward compatibility but are not able reflect variables that are intended to be not set on runtime.
As visible from the comment of #830 people struggle with "how to configure testcontainers".

Having to delay import to set a config for a lib is just not applicable on how people test things in python.
Using the dataclass seems much more applicable.

I suggest merging the PR and I hope I find time to proprose another PR that handles config runtime changable properly.

ryuk_privileged: bool = RYUK_PRIVILEGED
ryuk_disabled: bool = RYUK_DISABLED
ryuk_docker_socket: str = RYUK_DOCKER_SOCKET
# ryuk_docker_socket: str = RYUK_DOCKER_SOCKET
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# ryuk_docker_socket: str = RYUK_DOCKER_SOCKET
_ryuk_docker_socket: str = ""

Comment on lines +134 to +136
@property
def ryuk_docker_socket(self) -> str:
return get_docker_socket()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@property
def ryuk_docker_socket(self) -> str:
return get_docker_socket()
@property
def ryuk_docker_socket(self) -> str:
if not self._ryuk_docker_socket:
self.ryuk_docker_socket = get_docker_socket()
return self._ryuk_docker_socket
@ryuk_docker_socket.setter
def ryuk_docker_socket(self, value: str) -> None:
self._ryuk_docker_socket = value

Cache the result but make it changeable

@CarliJoy
Copy link
Contributor

See #833

@alexanderankin
Copy link
Member Author

closing in favor of #833

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

Labels

None yet

Projects

None yet
-->

Development

Successfully merging this pull request may close these issues.

Bug: Testcontainer fails to fetch server api version on 4.11 but works on 4.10

3 participants

0