-
-
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
Update oracle wait strategy for Healthcheck #2931
Update oracle wait strategy for Healthcheck #2931
Conversation
You said in #1292 that the official Dockerfile even provides a health check, so I think using the Testcontainers health check wait strategy would even be nicer and ideally more future proof, especially considering slightly altered user images. |
If we can be confident that the images include healthchecks, then yes I think this would be a good idea. |
4cb868f
to
eb899ec
Compare
Updated for healthcheck and tested locally. Though, I'd be hesitant to merge this until we have proper tests setup. |
Thanks, LGTM like this. |
eb899ec
to
707de29
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this. |
withConnectTimeoutSeconds(DEFAULT_CONNECT_TIMEOUT_SECONDS); | ||
addExposedPorts(ORACLE_PORT, APEX_HTTP_PORT); | ||
addExposedPorts(ORACLE_PORT, APEX_HTTP_PORT, OEM_EXPRESS_PORT); |
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.
Adding OEM_EXPRESS_PORT
is not really necessary for this PR, isn't it?
Ok, maybe I messed something up during merging, but currently, the tests fail. Will have to investigate this further. |
@kiview @OnnoHuijgen I believe this PR is no longer needed. This was being proposed before the |
Cool, thanks a lot for clarifying this @KyleAure (and in general for being up to date regarding this module, much appreciated). You can find some more context here: |
Wait untilDATABASE IS READY TO USE!
is shown in the logs before starting.Wait until container healthcheck reports
healthy
Fixes #1292