Allow to set Shared Memory size (shmSize) for GenericContainer by fzakaria · Pull Request #957 · testcontainers/testcontainers-java

fzakaria · 2018-11-05T19:28:31Z

Add the necessary code so that shmSize can be set on a GenericContainer.

This is especially useful for when starting Oracle docker images because they need at least 1GB of shared memory space.

docker run ... --shm-size=1g oracle/database:11.2.0.2-xe

Fixes #952

rnorth · 2018-11-05T20:09:38Z

Thanks @fzakaria.

Looks like CircleCI had an unrelated glitch downloading the gradle wrapper so I've restarted the build.

fzakaria · 2018-11-05T20:53:38Z

Thanks @rnorth -- looks to have been resolved.

fzakaria · 2018-11-05T23:22:33Z

I updated the PR to have shmSize be simply bytes instead of 'megabytes' to avoid mebibyte vs megabyte confusion.

fzakaria · 2018-11-06T04:37:49Z

Looks like travis-ci had a flaky failure pulling an artifact -- could someone restart.
I believe this PR is ready to merge; thanks.

core/src/main/java/org/testcontainers/containers/GenericContainer.java

kiview

I've restarted Travis and added some comments up for discussion.

core/src/test/java/org/testcontainers/junit/GenericContainerRuleTest.java

core/src/main/java/org/testcontainers/containers/GenericContainer.java

core/src/test/java/org/testcontainers/junit/GenericContainerRuleTest.java

fzakaria · 2018-11-06T15:37:21Z

Thanks for feedback @kiview -- I've addressed them with latest update.

moved to busybox + try-with-resources in the test
moved HostConfig to a buildHostConfig
removed extra assert

kiview

Great changes, thanks!
I found one more small thing in the Javadoc.

core/src/main/java/org/testcontainers/containers/GenericContainer.java

…ner.java Co-Authored-By: fzakaria <[email protected]>

core/src/main/java/org/testcontainers/containers/GenericContainer.java

…ner.java

fzakaria · 2018-11-06T18:25:20Z

Updated @kiview .

core/src/main/java/org/testcontainers/containers/GenericContainer.java

Co-Authored-By: fzakaria <[email protected]>

fzakaria · 2018-11-14T19:00:32Z

@kiview What's next step to get this merged & published?

kiview · 2018-11-15T06:43:14Z

Hey @fzakaria, sorry for letting this hanging, I think it's good to go.
I'll ping the others about final feedback and then we can merge.

core/src/main/java/org/testcontainers/containers/GenericContainer.java

fzakaria · 2018-11-15T22:08:23Z

@bsideup made changes requested.

fzakaria · 2018-11-24T16:16:42Z

Update on this ? Quite a lot of effort to get a helpful change in.
I might abandon the process soon.

kiview

Hi @fzakaria,
as said before, thanks for the PR.

I can understand your frustration, but please keep in mind, that we are working on Testcontainers in our spare time. As such, there might be periods of time, were each of us can't invest the time we'd like to ideally invest into the project.

I have requested a tiny change (remove blank lines to reduce the diff) and then I'll merge directly.

core/src/main/java/org/testcontainers/containers/GenericContainer.java

…ner.java Co-Authored-By: fzakaria <[email protected]>

fzakaria · 2018-11-26T15:19:53Z

@kiview thanks.
Sorry for the attitude -- other stuff that day was likely getting to me.

kiview · 2018-11-26T15:40:15Z

Travis failure unrelated, restarted.

kiview · 2018-11-26T19:41:44Z

@fzakaria merged, thanks a lot for the great PR as well as your patience 🙂

Farid Zakaria added 2 commits November 5, 2018 11:16

Added ability to set shmSize in GenericContainer

19cacb7

fix test

59f38b4

fzakaria requested review from bsideup, kiview and rnorth as code owners November 5, 2018 19:28

Farid Zakaria added 2 commits November 5, 2018 15:19

Changed shmSize to bytes -- to avoid mebi vs mega byte confusion

650b1bf

fixed bug from refactor

df236f7

testcontainers deleted a comment Nov 5, 2018

testcontainers deleted a comment from fzakaria Nov 5, 2018

kiview reviewed Nov 6, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

kiview requested changes Nov 6, 2018

View reviewed changes

Farid Zakaria added 2 commits November 6, 2018 07:33

Addressed feedback in pull-request

f18d13a

undo import expansion from intellij in GenericContainer

f7ba27a

kiview requested changes Nov 6, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

This was referenced Nov 6, 2018

Can't set the shm_size for oracle xe #952

Closed

Oracle XE container not started on MAC #861

Closed

Update core/src/main/java/org/testcontainers/containers/GenericContai…

245d820

…ner.java Co-Authored-By: fzakaria <[email protected]>

fzakaria commented Nov 6, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

Update core/src/main/java/org/testcontainers/containers/GenericContai…

0977259

…ner.java

kiview requested changes Nov 6, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

Change KB to MB in comments

0853a9b

Co-Authored-By: fzakaria <[email protected]>

kiview approved these changes Nov 6, 2018

View reviewed changes

bsideup reviewed Nov 15, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

move setting hostconfig to applyConfiguration

4ab6a9f

host config has to be at the start of applyConfigurations

7fdc516

kiview requested changes Nov 26, 2018

View reviewed changes

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

core/src/main/java/org/testcontainers/containers/GenericContainer.java Outdated Show resolved Hide resolved

kiview and others added 2 commits November 26, 2018 07:18

Update core/src/main/java/org/testcontainers/containers/GenericContai…

0347360

…ner.java Co-Authored-By: fzakaria <[email protected]>

Update core/src/main/java/org/testcontainers/containers/GenericContai…

e18c248

…ner.java Co-Authored-By: fzakaria <[email protected]>

kiview approved these changes Nov 26, 2018

View reviewed changes

kiview added this to the next milestone Nov 26, 2018

kiview merged commit 58a6c69 into testcontainers:master Nov 26, 2018

Uh oh!

Conversation

fzakaria commented Nov 5, 2018

Uh oh!

rnorth commented Nov 5, 2018

Uh oh!

fzakaria commented Nov 5, 2018

Uh oh!

fzakaria commented Nov 5, 2018

Uh oh!

fzakaria commented Nov 6, 2018

Uh oh!

Uh oh!

kiview left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fzakaria commented Nov 6, 2018

Uh oh!

kiview left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fzakaria commented Nov 6, 2018

Uh oh!

Uh oh!

fzakaria commented Nov 14, 2018

Uh oh!

kiview commented Nov 15, 2018

Uh oh!

Uh oh!

fzakaria commented Nov 15, 2018

Uh oh!

fzakaria commented Nov 24, 2018

Uh oh!

kiview left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fzakaria commented Nov 26, 2018

Uh oh!

kiview commented Nov 26, 2018

Uh oh!

kiview commented Nov 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants