You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/19 09:22:50 UTC
[GitHub] [cloudstack] shwstppr opened a new pull request #4604: api: add zone, vm name params in listVmSnapshot response
shwstppr opened a new pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604
### Description
Adds new params in listVmSnapshot API - `zonename`, `virtualmachinename`
Fixes #4603
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [x] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
UI
After change zone name and VM name are shown
CMK:
Before -
```
> list vmsnapshot vmsnapshotid=db83c1a7-7870-442d-8c27-d9d407a05f11
{
"count": 1,
"vmSnapshot": [
{
"account": "admin",
"created": "2021-01-20T17:34:35+0530",
"current": true,
"displayname": "vm-snap",
"domain": "ROOT",
"domainid": "52611b65-5b16-11eb-b67a-645d8651f45a",
"hypervisor": "Simulator",
"id": "db83c1a7-7870-442d-8c27-d9d407a05f11",
"name": "i-2-3-VM_VS_20210120120435",
"state": "Ready",
"tags": [],
"type": "Disk",
"virtualmachineid": "3a07d2d4-18ad-4ef4-ac97-7937ba629069",
"zoneid": "5918c672-47aa-4425-9ff9-d71d79d2b304",
}
]
}
```
After-
```
> list vmsnapshot vmsnapshotid=db83c1a7-7870-442d-8c27-d9d407a05f11
{
"count": 1,
"vmSnapshot": [
{
"account": "admin",
"created": "2021-01-20T17:34:35+0530",
"current": true,
"displayname": "vm-snap",
"domain": "ROOT",
"domainid": "52611b65-5b16-11eb-b67a-645d8651f45a",
"hypervisor": "Simulator",
"id": "db83c1a7-7870-442d-8c27-d9d407a05f11",
"name": "i-2-3-VM_VS_20210120120435",
"state": "Ready",
"tags": [],
"type": "Disk",
"virtualmachineid": "3a07d2d4-18ad-4ef4-ac97-7937ba629069",
"virtualmachinename": "test-vm",
"zoneid": "5918c672-47aa-4425-9ff9-d71d79d2b304",
"zonename": "Sandbox-simulator"
}
]
}
```
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764443315
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764617631
@shwstppr is this for 4.15 or master, pl change base branch or milestone
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766640468
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765124737
> > @shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
> > If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
> > I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
>
> @sureshanaparti I may agree with you on that partly but changes aren't causing any additional db query, plus we have similar parameters in other api responses so maybe this can be allowed ;)
> Calling list api just for names in UI will be an overkill
Agree with you @shwstppr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r561639905
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
okay @harikrishna-patnala
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764372597
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r575116315
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +622,14 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(Strings.isNullOrEmpty(vm.getDisplayName()) ? vm.getHostName() : vm.getDisplayName());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
@rhtyd that was by mistake in my previous commit :facepalm: . Fixed it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-778105644
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764372597
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764443718
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-778138601
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2686
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764372597
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764371683
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766641577
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r561656123
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
Thanks @shwstppr, looks good to me now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766797404
UI build: :heavy_check_mark:
Live QA URL: http://primate-qa.cloudstack.cloud:8080/client/pr/4604 (JID-3829)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r575026036
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +622,14 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(Strings.isNullOrEmpty(vm.getDisplayName()) ? vm.getHostName() : vm.getDisplayName());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
Why is setVirtualMachineName() added twice @shwstppr ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765192299
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766519108
> @shwstppr May be some generic approach for the API response objects, to tag both UUID and Name (wherever UUID is returned) might work for all the APIs, and the names can be used appropriately in the UI when required.
@sureshanaparti I get your proposal, but that I think it needs significant amount of changes as Name is not part of common interfaces (which I think is required for a generic approach) and it is defined as different names for each resource.
May be an another improvement ticket would help this PR to get continued !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r578392476
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +622,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(Strings.isNullOrEmpty(vm.getDisplayName()) ? vm.getHostName() : vm.getDisplayName());
Review comment:
LGTM - we need one check if for normal user/account it leaks the internal hostname (is hostname the i-x-y-VM?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766640953
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764448774
@shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-763572595
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766641476
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765298577
> > @shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
> > If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
> > I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
>
> @sureshanaparti I may agree with you on that partly but changes aren't causing any additional db query, plus we have similar parameters in other api responses so maybe this can be allowed
> Calling list api just for names in UI will be an overkill
@shwstppr So, do you mean it is the right approach to consider the API response object parameters for UI display? Few recent PRs below, with such changes where the response parameters are mainly updated for UI display only, and I'm sure this will repeat to replace most of the UUID displayed in the UI for the names. :-)
https://github.com/apache/cloudstack/pull/4483 - updated listNetworks API response for VPC Name
https://github.com/apache/cloudstack/pull/4275 - updated listVmSnapshot API response for Hypervisor
https://github.com/apache/cloudstack/pull/4073 - updated listPublicIpAddress API response for Network Name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765298577
> > @shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
> > If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
> > I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
>
> @sureshanaparti I may agree with you on that partly but changes aren't causing any additional db query, plus we have similar parameters in other api responses so maybe this can be allowed ;)
> Calling list api just for names in UI will be an overkill
@shwstppr So, do you mean it is the right approach to consider the API response object parameters for UI display? Few recent PRs below, with such changes where the response parameters are mainly updated for UI display only.
https://github.com/apache/cloudstack/pull/4483 - updated listNetworks API response for VPC Name
https://github.com/apache/cloudstack/pull/4275 - updated listVmSnapshot API response for Hypervisor
https://github.com/apache/cloudstack/pull/4073 - updated listPublicIpAddress API response for Network Name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764448774
@shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-763604492
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2555
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766641577
@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764443718
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-763573992
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766519108
> @shwstppr May be some generic approach for the API response objects, to tag both UUID and Name (wherever UUID is returned) might work for all the APIs, and the names can be used appropriately in the UI when required.
@sureshanaparti I get your proposal, but that I think it needs significant amount of changes as Name is not part of common interfaces (which I think is required for a generic approach) and it is defined as different names for each resource.
May be an another improvement ticket would help this PR to get continued !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764617631
@shwstppr is this for 4.15 or master, pl change base branch or milestone
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765573743
<b>Trillian test result (tid-3408)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35127 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4604-t3408-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_07_deploy_kubernetes_ha_cluster | `Failure` | 3619.44 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 64.07 | test_kubernetes_clusters.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764443315
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764371683
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765306573
@shwstppr May be some generic approach for the API response objects, to tag both UUID and Name (wherever UUID is returned) might work for all the APIs, and the names can be used appropriately in the UI when required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764628659
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-763572595
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766790175
@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764779066
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2568
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r579040576
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +622,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(Strings.isNullOrEmpty(vm.getDisplayName()) ? vm.getHostName() : vm.getDisplayName());
Review comment:
checked hostname would return name from the Db table, lgtm
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764448774
@shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765191876
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766639433
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r561628451
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
I think display_name should be first choice, if it is null then we can go with host_name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764481878
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2562
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765124737
> > @shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
> > If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
> > I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
>
> @sureshanaparti I may agree with you on that partly but changes aren't causing any additional db query, plus we have similar parameters in other api responses so maybe this can be allowed ;)
> Calling list api just for names in UI will be an overkill
Agree with you @shwstppr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764630797
> @shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
>
> If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
>
> I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
@sureshanaparti I may agree with you on that partly but changes aren't causing any additional db query, plus we have similar parameters in other api responses so maybe this can be allowed ;)
Calling list api just for names in UI will be an overkill
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764627979
@rhtyd sorry for the error. Changed the base branch
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766639433
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764371683
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r561628451
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
I think display_name should be first choice, if it is null then we can go with host_name.
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
Thanks @shwstppr, looks good to me now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-765306573
@shwstppr May be some generic solution for the API response objects, to tag both UUID and Name (wherever UUID is returned) might work for all the APIs, and the names can be used appropriately in the UI when required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766640468
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764372597
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766639433
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-763573992
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-778105168
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766789624
@blueorangutan ui
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764371683
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd removed a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
rhtyd removed a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-766639433
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#issuecomment-764448774
@shwstppr I do understand that it is good to display name(s) of resources/entities in the UI. But, I think, to change the respective API responses to include the name(s) of the required resources/entities (to show in the UI) may not be the right thing. The UUIDs can be further used with the list API(s) to get the required details (name, zone, etc) to fill the UI as desired.
If we continue this exercise to show name(s) in the UI, finally we end up adding name(s) to most of the responses.
I know UI has to attempt multiple API calls, but API responses should primarly return UUIDs. For names and other details, make use of list API calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on a change in pull request #4604: api: add zone, vm name params in listVmSnapshot response
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4604:
URL: https://github.com/apache/cloudstack/pull/4604#discussion_r561639905
##########
File path: server/src/main/java/com/cloud/api/ApiResponseHelper.java
##########
@@ -621,11 +621,13 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) {
vmSnapshotResponse.setDisplayName(vmSnapshot.getDisplayName());
UserVm vm = ApiDBUtils.findUserVmById(vmSnapshot.getVmId());
if (vm != null) {
- vmSnapshotResponse.setVirtualMachineid(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineId(vm.getUuid());
+ vmSnapshotResponse.setVirtualMachineName(vm.getHostName());
Review comment:
okay @harikrishna-patnala
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org