-
-
Notifications
You must be signed in to change notification settings - Fork 302
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(Oracle): Add support for WithDatabase(string) for Oracle 18 and onwards #1321
feat(Oracle): Add support for WithDatabase(string) for Oracle 18 and onwards #1321
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
0ade2f4
to
84f5181
Compare
11 Oracle containers from 4 images and the GitHub runner fails with No space left on device 😖 |
It looks like we are right at the edge. Although I expected ~20GB to be enough, I thought the root filesystem had something like ~30GB left. Maybe we can save some MBs by restoring the NuGet cache on the temp disk, but that likely won't be enough too.
What do you think about removing |
84f5181
to
71b1ffa
Compare
Use the correct database default name depending on the Docker image version and enable setting a custom database name for Oracle 18 and onwards. Fixes testcontainers#1233
00487a4
to
53ad636
Compare
In order to avoid "No space left on device" error on GitHub actions
2db8e62
to
588f89b
Compare
I tried something else that seems to work pretty well: splitting the Oracle tests into multiple projects so that we don't have the disk space issue. Let's hope this can land into version 4.3.0. Mac users will be happy to be able to use the I also just realised that the *-slim images are actually slim but that the *-slim-faststart images are not slim at all, what a misleading name!
So if you prefer to keep all the Oracle tests in the same project we might try the *-slim images instead of the *-slim-faststart images and we'll save quite some GBs! |
Cool idea utilizing the compiler constants 😎 I will try to review it tomorrow. |
The faststart images have performed the installation process already which probably will involves some unpacking, hence the bigger image. However, they start significantly faster which is key in integration tests. Would this non fast start only be used in your own tests or also as default in the shipped lib? if the latter, I would suggest to keep the slim fast start. |
The default image tag is |
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.
Thank you for the PR again. It looks good. I just have two minor questions. Let me know what you think and if we need to address them.
Older images that don't support a custom database will simply ignore it.
This reverts commit e17bd73. Container would fail to start with this error. > ORA-65012: Pluggable database FREEPDB1 already exists.
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.
Thanks ✌️.
What does this PR do?
This pull request selects the correct database default name depending on the Docker image version and enable setting a custom database name for Oracle 18 and onwards.
Why is it important?
This enables using the new Oracle Docker images (e.g. gvenzl/oracle-free) which doesn't work with the current implementation because of the hardcoded
XEPDB1
default database.Note
The
gvenzl/oracle-free
image is the first one with arm64 support which is especially interesting for macOS users.Related issues
How to test this PR
The
OracleContainerTest
class has been augmented to test many variations of the Oracle Docker images.