Skip to content
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

Merged
merged 5 commits into from Jul 16, 2019
Merged

Add DB2 module #1611

merged 5 commits into from Jul 16, 2019

Conversation

aguibert
Copy link
Contributor

This is a duplicate of PR #860 but it has been updated to address review comments and also use the newer ibmcom/db2 image.

@bsideup
Copy link
Member

bsideup commented Jul 14, 2019

@aguibert the DB2 JDBC URL test is failing:
https://circleci.com/gh/testcontainers/testcontainers-java/15833

@aguibert
Copy link
Contributor Author

@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.

@kiview
Copy link
Member

kiview commented Jul 15, 2019

Azure pipeline was hanging, I restarted it.

@aguibert
Copy link
Contributor Author

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.

@aguibert aguibert force-pushed the db2-newer-image branch 2 times, most recently from 9268397 to da1e16b Compare July 15, 2019 14:54
@bsideup
Copy link
Member

bsideup commented Jul 15, 2019

@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

@bsideup
Copy link
Member

bsideup commented Jul 15, 2019

@aguibert could you please revert the changes to the AZP config?

@aguibert
Copy link
Contributor Author

@bsideup according to this doc, since Testcontainers is a public repo we should be able to get a timeout of up to 6 hours:
https://docs.microsoft.com/en-us/azure/devops/pipelines/process/phases?view=azure-devops&tabs=yaml#timeouts

The 1 hour timeout would be if this was a private repo using Microsoft-hosted agents.

@aguibert
Copy link
Contributor Author

aguibert commented Jul 15, 2019

I am happy to not fiddle with the AZP config in this PR though 😃
I've reverted the changes to AZP config

@aguibert
Copy link
Contributor Author

thanks for reviewing @bsideup! All comments should be resolved now

Copy link
Member

@bsideup bsideup left a 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 🎉 👍

@aguibert
Copy link
Contributor Author

@bsideup looks like all checks except AZP are green now -- should be ready to merge!

@bsideup bsideup merged commit 3dc9d9d into testcontainers:master Jul 16, 2019
@bsideup
Copy link
Member

bsideup commented Jul 16, 2019

@aguibert merged 🎉 Thanks a lot for this contribution! 👍

@rnorth
Copy link
Member

rnorth commented Jul 16, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants