Skip to content

Add configs for Statistics API#957

Merged
KostyaSha merged 7 commits intodocker-java:masterfrom
yuting-liu:yuting-network-config
Dec 9, 2017
Merged

Add configs for Statistics API#957
KostyaSha merged 7 commits intodocker-java:masterfrom
yuting-liu:yuting-network-config

Conversation

@yuting-liu
Copy link
Copy Markdown
Contributor

@yuting-liu yuting-liu commented Nov 29, 2017 •
edited by marcuslinke
Loading

In given Docker Engine API, networks are given in Json as the structure of rx_bytes, rx_dropped, rx_errors, rx_packets, tx_bytes, tx_dropped, tx_errors, and tx_packets.

This change is Reviewable

@yuting-liu yuting-liu changed the title Add statistics network config for Statistics API Add configs for Statistics API Nov 29, 2017
@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 29, 2017 •
edited
Loading

Codecov Report

Merging #957 into master will increase coverage by 0.5%.
The diff coverage is 96.15%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #957     +/-   ##
========================================
+ Coverage   60.19%   60.7%   +0.5%     
========================================
  Files         421     429      +8     
  Lines        8383    8461     +78     
  Branches      538     538             
========================================
+ Hits         5046    5136     +90     
+ Misses       3040    3030     -10     
+ Partials      297     295      -2
Impacted Files Coverage Δ
...om/github/dockerjava/api/model/CpuStatsConfig.java 100% <100%> (ø)
...hub/dockerjava/api/model/ThrottlingDataConfig.java 100% <100%> (ø)
...github/dockerjava/api/model/MemoryStatsConfig.java 100% <100%> (ø)
.../github/dockerjava/api/model/BlkioStatsConfig.java 100% <100%> (ø)
...va/com/github/dockerjava/api/model/Statistics.java 90% <100%> (+78.88%) ⬆️
...om/github/dockerjava/api/model/CpuUsageConfig.java 100% <100%> (ø)
...a/com/github/dockerjava/api/model/StatsConfig.java 100% <100%> (ø)
...m/github/dockerjava/api/model/PidsStatsConfig.java 100% <100%> (ø)
.../dockerjava/api/model/StatisticNetworksConfig.java 75% <75%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e80848b...8337452. Read the comment docs.

@KostyaSha
Copy link
Copy Markdown
Member

KostyaSha commented Nov 29, 2017

@yuting-liu could you dump json file from live instance and implement serder test?

@yuting-liu
Copy link
Copy Markdown
Contributor Author

yuting-liu commented Nov 29, 2017 •
edited
Loading

@KostyaSha I'm not quite clear about your suggestions. Would you mind giving more details?
Do you mean that I should add a new StatisticTest along with this, or I'm not allowed to add this StatisticNetWorksConfig?

@KostyaSha
Copy link
Copy Markdown
Member

KostyaSha commented Nov 29, 2017

Something like this

docker-java/src/test/java/com/github/dockerjava/api/model/ContainerTest.java

Line 19 in 6d9dec3

public class ContainerTest {

Dump with wireshark or curl response, place in resources and roundtrip test for new fields

KostyaSha
KostyaSha reviewed Dec 6, 2017
View reviewed changes
src/test/java/com/github/dockerjava/api/model/StatisticsTest.java Outdated
assertThat(stat.getRead(), equalTo("2017-12-06T00:42:03.8352972Z"));

final StatisticNetWorksConfig networkConfig = stat.getNetworks().get("eth0");
assertThat(networkConfig.getRxBytes(), equalTo(1230L));
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 6, 2017

Choose a reason for hiding this comment

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

you can use is()

@KostyaSha KostyaSha added this to the 3.1.0 milestone Dec 9, 2017
@KostyaSha KostyaSha merged commit e110a4b into docker-java:master Dec 9, 2017
@KostyaSha
Copy link
Copy Markdown
Member

KostyaSha commented Dec 9, 2017

Thanks!

@yuting-liu
Copy link
Copy Markdown
Contributor Author

yuting-liu commented Jan 2, 2018

@KostyaSha I have a question about this. Since Docker API will change frequently, do we need to change these classes every time with the API update.

@yuting-liu yuting-liu mentioned this pull request Jan 3, 2018
Closed
@KostyaSha KostyaSha mentioned this pull request Sep 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KostyaSha KostyaSha KostyaSha left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.1.0-rc-1

Development

Successfully merging this pull request may close these issues.

3 participants

@yuting-liu @codecov-io @KostyaSha