-
Notifications
You must be signed in to change notification settings - Fork 296
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
feat(core): Extend container support #559
Conversation
00bd819
to
27dfc5a
Compare
core/testcontainers/core/image.py
Outdated
def __exit__(self, exc_type, exc_val, exc_tb) -> None: | ||
self.remove() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than untagging - why not never tag and let it be cleaned up by system prune
? im just thinking aloud - not 100% confident about how it works in java but the other pr for custom image also does what you did here, just challenging yall to think of something without the tagging/removal issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try and follow up on this notion (sorry for the wall of text)
let me start with saying I do think its possible, the tag in the image is not mandatory, and if kept as a ref to the image object we can probably use the id to be passed as the image
to DockerContainer
, this will have a very different workflow from what I presented in this commit as we don't really need a context manager. now having established this:
- as a concept I think tests should cleanup after themself
-- for example in the case where we ran similar tests one after the other, in our case a very similar image with small changes and I want to make sure it does get rebuilt in the process, you can argue that sometimes we do want to reuse the image but from my experience a clean slate is preferable.
--system prune
is great but I tend to keep it for when I ran out of space or more generally when something bad happened, as a user I don't want to run it after each test to keep a neat and clean env. (not saying it not possible ofc, just less preferable imo) - we can expanded this and draw similarities to the
DockerContainer
object. as part of its workflow onstop
it also does a targetedremove
, so I can argue we are simply keeping the same behavior for but containers and images. or do we consider maybe not removing the container at the end and also letting the user clean it later? (I know this is kind of a stretch) - regarding how
testcontainers-java
handle this, They use a more more elaborated system to handle images (for ref https://github.com/testcontainers/testcontainers-java/tree/d3b0df2d89ee349890c94b0ce220a167c82ddf51/core/src/main/java/org/testcontainers/images) I am not sure we can or should do exactly what they are doing (at least at this stage) I did invest some time digging the code (I'm no java expert) and they do have https://github.com/testcontainers/testcontainers-java/blob/d3b0df2d89ee349890c94b0ce220a167c82ddf51/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java#L55private boolean deleteOnExit = true;
and checking with the official docs https://java.testcontainers.org/features/creating_images/#automatic-deletion confirms they do intend on removing the temp images, but I can't pinpoint the exact location they are doing it (the only clue I have is that it happens on the JVM shutdown).
TL;DR
I think the tagging can be changed to be optional (need to do some testing) and the removal is not an issue but a very important feature (implemented and included on the java version) which can be optional (defaults to true)
Image creation is now based on id as advised by @alexanderankin (tag is optional and not critical to the build flow) |
Image cleanup is now optional (defaults to |
Please note that |
for fast api, i can imagine the plan is to create a Testcontainer for an intree service to test against from other intree services' test suites. what is the plan for aws lambda exactly? |
I think its basically extending the paradigm for with RedisContainer() as redis_container:
redis_params = redis_container.get_client_params()
with FastAPIContainer() as server: # this is very close to SrvContainer
# do some calls to the server and utilize the fact that its connected to redis *at this point I'm not as sure we will need a module for FastAPI but still needs to think about it Now regarding with LocalStackContainer() as localstack:
dynamo_client = localstack.get_client("dynamodb")
# prepare something in dynamo
with AwsLambdaContainer() as lambda:
# run some requests to http://localhost:9000/2015-03-31/functions/function/invocations
# this way we can be validating flow with dynamo
# and maybe even checking the changes on dynamo itself *this is static URL in all AWS lambdas |
Decided to implement a module for FastAPI, I think it will be good even on a didactic level for anyone who needs to use it, as it include a working example and all the sources. |
Added DockerImage to core Make sure all build related stuff are removed after Fix Image related issues Improve image removal code Add test from docker client kwargs image Added core test from image flow Improve docstring for Image Update core Readme to include image and full example Update core Readme to include image and full example Fix usage of image name Update test from image string Replace image name with tag Update image remove handling Fix image usage doctest
Added doctest to CustomContainer and update readme Fix issue with CustomContainer image handle Add test for CustomContainer Update image var on CustomContainer Image build based on id Image clean is now optional Fixed doctest Update log about image removal Improve image related test flow Refactor CustomContainer into SrvContainer Refactor image related test flow Improve SrvContainer Fix test_srv_container logs check Improve SrvContainer and update tests Updates for SrvContainer
Fix missing doc in index Fix extra Fix missing httpx as optional dependency
f4490c2
to
5088097
Compare
They now reflect better the improvements and the work that was done. |
I think I should split this PR to the 4 commit (PR for each)
So it will be easier to review and keep track |
@Tranquility2 Thank you for your work here and joining efforts with the project, much appreciated 🙏. I agree with your last proposal, splitting these changes into 4 dedicated PR would be much better. Fun fact: That is also how I originally joined the Testcontainers project as a maintainer, since I authored a very similar project in Java back in the day 😉 |
1st part is up: #585 |
As part of the effort described, detailed and presented on #559 (Providing the implementation for #83 - Docker file support and more) This is the first PR (out of 4) that should provide all the groundwork to support image build. This would allow users to use custom images: ```python with DockerImage(path=".") as image: with DockerContainer(str(image)) as container: # Test something with/on custom image ``` Next in line is: `feat(core): Added SrvContainer` And later on: `feat(core): Added FastAPI module` `feat(core): Added AWS Lambda module` (all of the above can be overviewed on #559)
2nd part is up: #595 |
As part of the effort described, detailed and presented on #559 This is the seconds PR (out of 4) that should provide all the groundwork to support containers running a server. This would allow users to use custom images: ```python with DockerImage(path=".", tag="test:latest") as image: with ServerContainer(port=9000, image=image) as srv: # Test something with/on the server using port 9000 ``` Next in line are: `feat(core): Added FastAPI module` `feat(core): Added AWS Lambda module` --- Based on the work done on #585 Expended from issue #83 --------- Co-authored-by: David Ankin <[email protected]>
As part of the effort described, detailed and presented on #559 This is the third PR (out of 4) that should provide all the groundwork to support containers running a server. As discussed on #595 this PR aims to refactor the `ServerContainer` under a new dedicated module called "generic". ![image](https://github.com/testcontainers/testcontainers-python/assets/7189138/b7a3395b-ce3c-40ef-8baa-dfa3eff1b056) The idea is that this module could include multiple generic implementations such as ```server.py``` with the proper documentation and examples to allow users simpler usage and QOL. This PR adds the original FastAPI implementation as a simple doc example, I think this aligns better following #595 Next in line is ```feat(core): Added AWS Lambda module``` Based on the work done on #585 and #595 Expended from issue #83 --- Please note an extra commit is included to simulate the relations when importing between and with other modules.
4th part is up: #655 |
As part of the effort described, detailed and presented on #559 This is the 4th (and final in this track) PR that should provide support for AWS Lambda containers. This module will add the ability to test and run Amazon Lambdas (using the built-in runtime interface emulator) For example: ```python from testcontainers.aws import AWSLambdaContainer from testcontainers.core.waiting_utils import wait_for_logs from testcontainers.core.image import DockerImage with DockerImage(path="./modules/aws/tests/lambda_sample", tag="test-lambda:latest") as image: with AWSLambdaContainer(image=image, port=8080) as func: response = func.send_request(data={'payload': 'some data'}) assert response.status_code == 200 assert "Hello from AWS Lambda using Python" in response.json() delay = wait_for_logs(func, "START RequestId:") ``` This can (and probably will) be used with the provided [LocalStackContainer](https://testcontainers-python.readthedocs.io/en/latest/modules/localstack/README.html) to help simulate more advance AWS cases. --- Based on the work done on: - #585 - #595 - #612 Expended from issue #83
Overview
So basically as you can see from the title this adds a functionally that I think is super useful, supporting working with Dockerfile for achieving even more compatibility with custom images.
The created image is cleaned and any any intermediate containers are removed.
Background
I'm a bit biased as I originally started from working on my own project: https://github.com/Tranquility2/dockerr that has some very clear similarities with
testcontainers
.I noticed
testcontainers
in Go and Java (for example) indeed support Dockerfile (and even some extra compatibility modes like Dockerfile DSL) so decided to take the time and add this totestcontainers-python
as well.Goal
The end goal will be supporting custom images that can be tested, in my use case:
Work Plan
This is what I'm planning to do
get_api
to get the full user experienceFuture work
Any feedback is welcomed and I'll be happy to update/fix as needed.
Update
Took me some time but I did found #83 which is quite old and #455 which takes a different approach from what I have here.
Update 2 - Please note this proposal emphasize on 2 things
DockerContainer
basically remains the same, this limits maintenance required and allows for easier integration.Update 3 - Hopefully done ,this PR includes multiple feats