-
-
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
Add DB2 module #1611
Add DB2 module #1611
Conversation
a98e960
to
5e6e49b
Compare
@aguibert the DB2 JDBC URL test is failing: |
5e6e49b
to
0be7ef0
Compare
@bsideup thanks for the heads up. I ran this test before the PR and it passed locally somehow 🤔 In any case, was able to reproduce the failure and fix up the test. Should be all set now. |
Azure pipeline was hanging, I restarted it. |
Looks like the pipeline is hitting the 60m timeout, it is possible that the 2 additional DB2 tests have pushed it over the edge. The last successful Azure pipeline build was just barely under the timeout at 58m8s. These 2 new DB2 tests add 2 more DB2 container starts, which take ~90s each. |
9268397
to
da1e16b
Compare
@aguibert it does not make sense to increase the timeouts because there is a hard limit from Azure hosted nodes. I will split them into smaller, parallel chunks later today. For now, you can ignore the Azure Linux one and pay attention to Travis/CircleCI. Sorry for the inconvenience |
@aguibert could you please revert the changes to the AZP config? |
@bsideup according to this doc, since Testcontainers is a public repo we should be able to get a timeout of up to 6 hours: The 1 hour timeout would be if this was a private repo using Microsoft-hosted agents. |
da1e16b
to
e969b61
Compare
I am happy to not fiddle with the AZP config in this PR though 😃 |
modules/db2/src/main/java/org/testcontainers/containers/Db2Container.java
Outdated
Show resolved
Hide resolved
modules/jdbc-test/src/test/java/org/testcontainers/junit/SimpleDb2Test.java
Show resolved
Hide resolved
modules/jdbc-test/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java
Outdated
Show resolved
Hide resolved
thanks for reviewing @bsideup! All comments should be resolved now |
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.
Looks good! Let's merge this bad boy once CI is green 🎉 👍
@bsideup looks like all checks except AZP are green now -- should be ready to merge! |
@aguibert merged 🎉 Thanks a lot for this contribution! 👍 |
Awesome! Thank you, @aguibert for this! Let's try and ship a release with this soon, even if it's a fairly light release otherwise. |
This is a duplicate of PR #860 but it has been updated to address review comments and also use the newer
ibmcom/db2
image.