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
Add sent and received bytes to listNetworks and listVirtualMachines. #4776
Conversation
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.
clgtm needs testing, can you add info about the use-case to the description or refer an issue (even though it is trivial, it might hekp tracking if someone asks about this)
@DaanHoogland added more details in description |
Packaging result: ✖️ centos7 ✖️ centos8 ✖️ debian. SL-JID 242 |
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.
server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java
Outdated
Show resolved
Hide resolved
Packaging result: ✖️ centos7 ✖️ centos8 ✖️ debian. SL-JID 251 |
@ravening Can you fix the conflicts? Thanks |
Hi @ravening there is still a conflict with this PR can you please resolve it? |
73a6816
to
5883f79
Compare
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 682 |
@blueorangutan test |
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1418)
|
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.
LGTM
Tested on VM on shared network:
(localcloud) SBCM5> > list networks filter=id,receivedbytes,sentbytes,
{
"count": 1,
"network": [
{
"id": "8d4184b3-6e76-4fc6-bbdb-16e387a6efbe",
"receivedbytes": 13519,
"sentbytes": 9444
}
]
}
(localcloud) SBCM5> > list virtualmachines filter=id,receivedbytes,sentbytes,
{
"count": 1,
"virtualmachine": [
{
"id": "1b954319-8a65-448b-bd1d-78f51ee2cd72",
"receivedbytes": 13519,
"sentbytes": 9444
}
]
}
api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.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.
Code LGTM.
But to me seems like you are using the key 'final' where we don't need it.
Did I understood something wrong?
Do we need more manual test on this PR? since @nvazquez has already did some tests.
api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java
Outdated
Show resolved
Hide resolved
It is, in some circles, good practice to make sure a parameter is only changed consciously. A bit overdone indeed, i would agree. |
Hi @ravening can you please address the open comments? |
Display the traffic data in networks and vm api response
359b0d7
to
8ae5ca9
Compare
@blueorangutan package |
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 812 |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1560)
|
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@blueorangutan package |
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 875 |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1649)
|
Description
Display the traffic data in networks and vm api response so that we can quickly get the traffic statistics about all the vm's in that network. User can traffic data for a particular network through api as well.
Also users can monitor the traffic on individual Vm as well which was not possible earlier.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?