Skip to content

add support for PidsLimit #734#764

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

add support for PidsLimit #734#764
KostyaSha merged 6 commits intodocker-java:masterfrom
msekunda:support-for-pids-limit-issue-734

Conversation

@msekunda
Copy link
Copy Markdown
Contributor

@msekunda msekunda commented Dec 15, 2016

Changes for #734

This change is Reviewable

@cirocosta
Copy link
Copy Markdown
Contributor

cirocosta commented Jan 5, 2017

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.

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha left a comment

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 :).

Copy link
Copy Markdown
Contributor

@marcuslinke marcuslinke left a comment

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