search

testcontainers / testcontainers-python

Testcontainers is a Python library that providing a friendly API to run Docker container. It is designed to create runtime environment to use during your automatic tests.
https://testcontainers-python.readthedocs.io/en/latest/
Apache License 2.0
1.44k stars 270 forks source link

Feature: Non-context-manager API for Network #627

Closed dogukancagatay closed 3 weeks ago

dogukancagatay commented 3 weeks ago

What are you trying to do?

I was writing some pytest fixtures to create containers, and I wanted to create a Network in a fixture. However, I realized that a non-context-manager interface wasn't provided for the Network class. I've already made the change here: https://github.com/dogukancagatay/testcontainers-python/pull/1

Why should it be done this way?

There is a non-context-manager API for other classes, such as DockerContainer and DockerImage, and I think that Network should have one for completeness, flexibility, and ease of use in some instances.

Other references:

Include any other relevant reading material about the enhancement.

alexanderankin commented 3 weeks ago

can you share your code for the fixture? normally you can yield from the fixture and then once the test suite is done, it runs the rest of your function, like __exit__ and such -- besides, why do you want to programmatically create a network that you do not also programmatically destroy?

dogukancagatay commented 3 weeks ago

I could have created Network with the context manager on my fixture. DockerContainer and DockerImage allows to be used outside context-manager, so why not Network have the non-context-manager way.

alexanderankin commented 3 weeks ago

does ryuk clean it up at least?

alexanderankin commented 3 weeks ago

ok, ive thought about this a bit, makes sense to allow this API - I would consider it a bug also if API's were not uniform in the way you described

additionally took this opportunity to review the code and discover we are not auto-cleaning up the networks so I fixed this as well.

so i feel like it is ready to merge now once ci passes.