-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
WIP Update oracleContainer to use new password and serviceName #2357
WIP Update oracleContainer to use new password and serviceName #2357
Conversation
Since we don't have CI for Oracle, I will built the Docker images on my machine and test by hand. |
Is this to be expected? |
This is not something I would expect. I have been testing against an 11g-R2 container and have not run into this. Let me see if I can replicate this using 18c. |
@kiview Looks like I ran into the same error here:
Output from the container indicated that the database had only installed up to 34%
I am guessing this is likely due to our wait strategy. We wait for a connection to become available then attempt to call This may be related to #1292 |
@KyleAure thanks for investigating this. In the light of #1292 and the fact that we have official images from Oracle, I would propose fixing the I can take care of this. But this would probably break backwards compatibility to the old (which is banned after all). |
Got kind of working waiting strategy, but the image takes 8 minutes to start. Does this even make sense in the context of TC? public OracleContainer(String dockerImageName) {
super(dockerImageName);
this.waitStrategy = new LogMessageWaitStrategy()
.withRegEx("DATABASE IS READY TO USE!\\n")
.withStartupTimeout(Duration.of(12, MINUTES));
} |
Yeah I would be hesitant to break compatibility with oracleinanutshell/oracle-xe-11g and wnameless/docker-oracle-xe-11g since I am sure most people are using those right now. But I think we should definitely document in our instructions how users can create their own images, and that we recommend building an image with a pre-built database (to alleviate startup time). If they are following those instructions they can set a password when building their image (in which case this PR is kind of moot). Thoughts? |
In the light of https://twitter.com/geraldvenzl/status/1230673245193539585?s=21 I would prefer to push for the official image. I would personally accept to break compatibility with the The question is if we try to make the new self=built Oracle image good to use (with e.g. including container volume binding instructions), or if we should wait for the new official image and hope it has faster startup times. |
Regarding startup times I could imagine a completely different approach for Oracle - see #2369 |
@KyleAure In the light of these facts, do you think it is okay to close the current PR? We currently plan to provide a testcontainer oracle-xe image on quay.io that contains an initialized database. I'd prefer if we continue from there. The change for the SID we would incorporate however to be compatbile. |
Yes, I agree. Sorry for the runaround. I am glad to see oracle is officially releasing something! I think if anything we should document that using |
Fixes #2356
Fixes #2215