Add CustomContainer exposing settings through the class

[vikahl]

Add a CustomContainer exposing the settings through the class.

Close #236

[tillahoffmann]

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?

[vikahl]

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.

[tillahoffmann]

The image name can be specified for DockerContainer here.

testcontainers-python/testcontainers/core/container.py

Lines 12 to 13 in 7346345

	class DockerContainer(object):
	def __init__(self, image, docker_client_kw: dict = None, **kwargs):

[vikahl]

The image name can be specified for DockerContainer here.

testcontainers-python/testcontainers/core/container.py

Lines 12 to 13 in 7346345

	class DockerContainer(object):
	def __init__(self, image, docker_client_kw: dict = None, **kwargs):

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.

[tillahoffmann]

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.

[vikahl]

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.

[vikahl]

@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.

[gaby]

Ping @tillahoffmann this is another great PR

[tillahoffmann]

Looks good, I left a few small comments. If you could add tests, that'd be great! 🙏

[vikahl]

Will take a look and add tests and do the updates when I am back at a computer (might take a few weeks)

[vikahl]

@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.

[rhoban13]

This PR seems to have stalled, but is a really good idea. What do we need to do to get it over the finish line?

[rhoban13]

Can we do the same with network and network_aliases? I suspect we need to rebase this PR in order to pickup these attributes.

[vikahl]

This PR seems to have stalled, but is a really good idea. What do we need to do to get it over the finish line?

It's been a while, but it's still a feature I would like to have (now I use a CustomContainer in my code base that does this).
The package code has changed a bit, but I will take a look at either rebase/merge the changes or just redo them on an updated master.

[rhoban13]

This PR seems to have stalled, but is a really good idea. What do we need to do to get it over the finish line?

It's been a while, but it's still a feature I would like to have (now I use a CustomContainer in my code base that does this). The package code has changed a bit, but I will take a look at either rebase/merge the changes or just redo them on an updated master.

Let me know if I can help - I can attempt a rebase and PR your branch or honestly your idea of just resubmitting might be just as easy.

[Tranquility2]

Hi @vikahl and @rhoban13, we had this talk or similar notion/idea on #559, we ended up with https://github.com/testcontainers/testcontainers-python/blob/main/modules/generic/testcontainers/generic/server.py which emphasis the notion to do custom stuff as a module (outside of core).
We also added the ability to utilize custom images as part of this track.
This can be improved of course (but in the module context) so hopefully it works for your use case and if not we can think how to extend it :)

[rhoban13]

I might suggest resubmitting this PR separately as the original intent sounds just like the ServerContainer which @Tranquility2 linked above. The current state of this PR is simply making the initializer of DockerContainer accept it's private members as kwargs.

[rhoban13]

I resubmitted as the above PR on an updated branch in hopes of getting it landed. I'm not sure who I need to reach out to to kick off the validations and get a review.

[alexanderankin]

prefer #809 but will leave both open until i have time to review + merge

[vikahl]

@rhoban13 thank you. I never got around to do it so happy that you did.

I'm leaving this PR open to be closed by the repo maintainers but #809 is a better/newer implementation of the idea behind this PR.