testcontainers
Add CustomContainer exposing settings through the class
Great, thank you! Slight misunderstanding: I was thinking that we could add this functionality to existing containers rather than create a new one. What do you think?
Great, thank you! Slight misunderstanding: I was thinking that we could add this functionality to existing containers rather than create a new one. What do you think?
I don't there is any current class (except for the deprecated TestContainer, but probably best to not touch it) that allows specifying the image name. I build the service image prior to tests so I have e.g., my-service:version1 as an image and want to manually specify that.
The image name can be specified for DockerContainer here.
https://github.com/testcontainers/testcontainers-python/blob/7346345d3636c383e6a685fc80b67c5947efd9fe/testcontainers/core/container.py#L12-L13
The image name can be specified for
DockerContainerhere.https://github.com/testcontainers/testcontainers-python/blob/7346345d3636c383e6a685fc80b67c5947efd9fe/testcontainers/core/container.py#L12-L13
True, you have a point there.
Just to clarify, do you suggest extending the __init__ method of DockerContainer or all the db-containers?
I think the DockerContainer approach would be convenient and won't require a new class. I can test the approach tomorrow and see if it would impact existing implementations.
Just to clarify, do you suggest extending the init method of DockerContainer or all the db-containers?
I was thinking all the containers as they all inherit from DockerContainer. That would give us a consistent interface across the whole library.
Just to clarify, do you suggest extending the init method of DockerContainer or all the db-containers?
I was thinking all the containers as they all inherit from
DockerContainer. That would give us a consistent interface across the whole library.
Ah, good thought. Sorry for not picking up on this but I'll update the PR.
@tillahoffmann updated the PR to instead set these in the core container's __init__. Do you think this looks reasonable?
If so, I'll make sure these changes are covered by tests as well.
Will take a look and add tests and do the updates when I am back at a computer (might take a few weeks)
@tillahoffmann please take a look at the tests.
I added test that tests that attributes can be set through the init function and later read through class attr.
The test does not test that the attributes has an actual effect, in the future it might be of interest to test the integration to the docker container and ensure that class attributes are propagated properly. However, I for the changes in this PR I think the current is good enough.