Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dogukancagatay opened this issue Jun 30, 2024 · 4 comments · Fixed by #630
Closed

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

dogukancagatay opened this issue Jun 30, 2024 · 4 comments · Fixed by #630

Comments

@dogukancagatay
Copy link
Contributor

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: dogukancagatay#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.

dogukancagatay added a commit to dogukancagatay/testcontainers-python that referenced this issue Jun 30, 2024
This PR adds `create` method to the `Network` class to enable
a non-context-manager usage.

Fixes testcontainers#627
@alexanderankin
Copy link
Collaborator

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
Copy link
Contributor Author

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 pushed a commit to dogukancagatay/testcontainers-python that referenced this issue Jul 1, 2024
This PR adds `create` method to the `Network` class to enable
a non-context-manager usage.

Fixes testcontainers#627
@alexanderankin
Copy link
Collaborator

does ryuk clean it up at least?

@alexanderankin
Copy link
Collaborator

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.

alexanderankin pushed a commit that referenced this issue Jul 2, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.7.1](testcontainers-v4.7.0...testcontainers-v4.7.1)
(2024-07-02)


### Bug Fixes

* **core:** bad rebase from
[#579](#579)
([#635](#635))
([4766e48](4766e48))
* **modules:** Mailpit Container
([#625](#625))
([0b866ff](0b866ff))
* **modules:** SFTP Server Container
([#629](#629))
([2e7dbf1](2e7dbf1))
* **network:** Now able to use Network without context, and has labels
to be automatically cleaned up
([#627](#627))
([#630](#630))
([e93bc29](e93bc29))
* **postgres:** get_connection_url(driver=None) should return
postgres://...
([#588](#588))
([01d6c18](01d6c18)),
closes
[#587](#587)
* update test module import
([#623](#623))
([16f6ca4](16f6ca4))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants