Skip to content

add support for PidsLimit #734 - #764

Merged
KostyaSha merged 6 commits into
docker-java:masterfrom
msekunda:support-for-pids-limit-issue-734
Jan 17, 2017
Merged

KostyaSha merged 6 commits into
docker-java:masterfrom
msekunda:support-for-pids-limit-issue-734

Conversation

@msekunda

@msekunda msekunda commented Dec 15, 2016

Copy link
Copy Markdown
Contributor

Changes for #734

This change is Reviewable

@cirocosta

cirocosta commented Jan 5, 2017

Copy link
Copy Markdown
Contributor

Any news on this? It looks fine


InspectContainerResponse inspectContainerResponse = dockerClient.inspectContainerCmd(container.getId()).exec();

assertEquals(inspectContainerResponse.getHostConfig().getPidsLimit(), hostConfig.getPidsLimit());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use hamcrest matcher.

@Override
public CreateContainerCmd withPidsLimit(Long pidsLimit) {
checkNotNull(pidsLimit, "pidsLimit was not specified");
hostConfig.withPidsLimit(pidsLimit);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the third PR... You can configure hostConfig directly without proxy calls from create object.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So even in testcase you create and configure hostConfig directly without calling proxy methods.

@KostyaSha KostyaSha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see no sense in proxying

@KostyaSha

Copy link
Copy Markdown
Member

And we end that it already supported and PR has only style formatting?

@KostyaSha

Copy link
Copy Markdown
Member

Sorry, now looks good :)

@msekunda

Copy link
Copy Markdown
Contributor Author

Should be fine, sorry for formatting problem :).

@marcuslinke marcuslinke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KostyaSha
KostyaSha merged commit 3ad75e0 into docker-java:master Jan 17, 2017
@KostyaSha KostyaSha added this to the 3.0.7 milestone Jan 17, 2017
panuse pushed a commit to TuKangTech/docker-java that referenced this pull request Aug 20, 2017
* add support for PidsLimit docker-java#734

* add support for PidsLimit docker-java#734  - using hamcrest in assertions

* add support for PidsLimit docker-java#734  - remove proxing

* add support for PidsLimit docker-java#734  - remove proxing

* add support for PidsLimit docker-java#734  - formatting fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants