Add CustomContainer exposing settings through the class #238

vikahl · 2022-08-25T13:33:51Z

Add a CustomContainer exposing the settings through the class.

Close #236

tillahoffmann · 2022-08-25T14:58:12Z

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 · 2022-08-25T15:12:31Z

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 · 2022-08-25T17:15:14Z

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 · 2022-08-25T19:29:19Z

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 · 2022-08-30T14:17:03Z

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 · 2023-03-16T08:43:46Z

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 · 2023-03-22T19:57:57Z

@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 · 2023-06-08T04:12:06Z

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! 🙏

core/testcontainers/core/container.py

vikahl · 2023-06-09T14:30:44Z

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

Ref testcontainers#236

Co-authored-by: Till Hoffmann <tillahoffmann@gmail.com>

Add test that tests that attributes can be set through the __init__ function and later read through class attr. This 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.

vikahl · 2023-06-27T17:22:03Z

@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 · 2025-04-02T16:08:06Z

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 · 2025-04-02T16:08:52Z

core/testcontainers/core/container.py

-        self.env = {}
-        self.ports = {}
-        self.volumes = {}
+    def __init__(


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 · 2025-04-02T18:36:35Z

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 · 2025-04-11T20:22:27Z

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 · 2025-04-11T21:48:42Z

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 · 2025-04-12T17:50:02Z

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 · 2025-05-06T13:38:39Z

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 · 2025-05-06T13:46:37Z

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

vikahl · 2025-05-06T18:28:00Z

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

@vikhal

…args (#809) Re submitting what is the end result of the iterations in #238 submitted originally by @vikhal. Simply enabling the initializer of `DockerContainer` to accept its private members as kwargs. --------- Co-authored-by: David Ankin <daveankin@gmail.com>

@vikhal

…args (testcontainers#809) Re submitting what is the end result of the iterations in testcontainers#238 submitted originally by @vikhal. Simply enabling the initializer of `DockerContainer` to accept its private members as kwargs. --------- Co-authored-by: David Ankin <daveankin@gmail.com>

vikahl changed the title ~~Add pyenv file to gitignore~~ Add CustomContainer exposing settings through the class Aug 25, 2022

vikahl marked this pull request as ready for review August 25, 2022 13:34

tillahoffmann added 👀 requires attention 📦 package: core labels Feb 16, 2023

vikahl force-pushed the new-generic-container branch from 6e3291a to 8622477 Compare March 22, 2023 19:56

tillahoffmann reviewed Jun 8, 2023

View reviewed changes

core/testcontainers/core/container.py Outdated Show resolved Hide resolved

core/testcontainers/core/container.py Outdated Show resolved Hide resolved

vikahl and others added 3 commits June 27, 2023 16:39

Update core container so that settings can be set in init

f280854

Ref testcontainers#236

Update core/testcontainers/core/container.py after tillahoffmanns review

b1b4811

Co-authored-by: Till Hoffmann <tillahoffmann@gmail.com>

Update docstring to Google style

2c4eb3b

vikahl force-pushed the new-generic-container branch from ac8b58f to 2c4eb3b Compare June 27, 2023 14:39

vikahl added 2 commits June 27, 2023 19:13

Fix errors in the DockerContainer class

45428d0

vikahl requested a review from tillahoffmann October 8, 2023 18:46

rhoban13 reviewed Apr 2, 2025

View reviewed changes

rhoban13 mentioned this pull request May 2, 2025

feat: DockerContainer initializer to accept its private members as kwargs #809

Merged

vikahl closed this May 28, 2025

Add CustomContainer exposing settings through the class #238

Add CustomContainer exposing settings through the class #238

Uh oh!

Conversation

vikahl commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tillahoffmann commented Aug 25, 2022

Uh oh!

vikahl commented Aug 25, 2022

Uh oh!

tillahoffmann commented Aug 25, 2022

Uh oh!

vikahl commented Aug 25, 2022

Uh oh!

tillahoffmann commented Aug 30, 2022

Uh oh!

vikahl commented Mar 16, 2023

Uh oh!

vikahl commented Mar 22, 2023

Uh oh!

gaby commented Jun 8, 2023

Uh oh!

tillahoffmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vikahl commented Jun 9, 2023

Uh oh!

vikahl commented Jun 27, 2023

Uh oh!

rhoban13 commented Apr 2, 2025

Uh oh!

rhoban13 Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

vikahl commented Apr 2, 2025

Uh oh!

rhoban13 commented Apr 11, 2025

Uh oh!

Tranquility2 commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhoban13 commented Apr 12, 2025

Uh oh!

rhoban13 commented May 6, 2025

Uh oh!

alexanderankin commented May 6, 2025

Uh oh!

vikahl commented May 6, 2025

Uh oh!

Uh oh!

vikahl commented Aug 25, 2022 •

edited

Loading

Tranquility2 commented Apr 11, 2025 •

edited

Loading