-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow to set Shared Memory size (shmSize) for GenericContainer #957
Conversation
Thanks @fzakaria. Looks like CircleCI had an unrelated glitch downloading the gradle wrapper so I've restarted the build. |
Thanks @rnorth -- looks to have been resolved. |
I updated the PR to have shmSize be simply bytes instead of 'megabytes' to avoid mebibyte vs megabyte confusion. |
Looks like travis-ci had a flaky failure pulling an artifact -- could someone restart. |
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
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.
I've restarted Travis and added some comments up for discussion.
core/src/test/java/org/testcontainers/junit/GenericContainerRuleTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/testcontainers/junit/GenericContainerRuleTest.java
Outdated
Show resolved
Hide resolved
Thanks for feedback @kiview -- I've addressed them with latest update.
|
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.
Great changes, thanks!
I found one more small thing in the Javadoc.
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
…ner.java Co-Authored-By: fzakaria <farid.m.zakaria@gmail.com>
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
Updated @kiview . |
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
Co-Authored-By: fzakaria <farid.m.zakaria@gmail.com>
@kiview What's next step to get this merged & published? |
Hey @fzakaria, sorry for letting this hanging, I think it's good to go. |
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
@bsideup made changes requested. |
Update on this ? Quite a lot of effort to get a helpful change in. |
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.
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
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/GenericContainer.java
Outdated
Show resolved
Hide resolved
…ner.java Co-Authored-By: fzakaria <farid.m.zakaria@gmail.com>
…ner.java Co-Authored-By: fzakaria <farid.m.zakaria@gmail.com>
@kiview thanks. |
Travis failure unrelated, restarted. |
@fzakaria merged, thanks a lot for the great PR as well as your patience 🙂 |
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.
Fixes #952