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/08 15:48:17 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4666: Fix bug in creating shared network
ravening opened a new pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666
### Description
Sometimes creating shared network fails because the physical network
data is null and redereferencing it causes NPE.
Wait till the physical network data is available before accessing it
Fixes https://github.com/apache/cloudstack/issues/4664
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- 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)
- [ ] 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
- [X] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [X] Critical
- [ ] Major
- [ ] Minor
- [ ] 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. -->
<!-- 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] blueorangutan commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-811703833
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4666 (SL-JID-83)
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-809279501
@rhtyd 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 commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-813235403
@davidjumani are you lgtm on merging this?
@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] davidjumani commented on a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r602022693
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -736,6 +736,7 @@ export default {
message: this.$t('message.request.failed'),
description: this.$t('message.error.add.guest.network')
})
+ return;
Review comment:
@ravening
```
/jenkins/workspace/acs-pr-ui-builder/ui/src/views/network/CreateSharedNetworkForm.vue
739:17 error Extra semicolon semi
```
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-801003782
@ravening any update on this?
I was able to not see mentioned issue with this,
```
diff --git a/ui/src/views/network/CreateSharedNetworkForm.vue b/ui/src/views/network/CreateSharedNetworkForm.vue
index e91a5631c4..eaaa38c083 100644
--- a/ui/src/views/network/CreateSharedNetworkForm.vue
+++ b/ui/src/views/network/CreateSharedNetworkForm.vue
@@ -567,7 +567,7 @@ export default {
var networks = json.listphysicalnetworksresponse.physicalnetwork
if (this.arrayHasItems(networks)) {
for (const i in networks) {
- this.addPhysicalNetworkForGuestTrafficType(networks[i], i * 1 === networks.length - 1)
+ this.addPhysicalNetworkForGuestTrafficType(networks[i])
}
} else {
this.formPhysicalNetworkLoading = false
@@ -576,7 +576,7 @@ export default {
})
}
},
- addPhysicalNetworkForGuestTrafficType (physicalNetwork, isLastNetwork) {
+ addPhysicalNetworkForGuestTrafficType (physicalNetwork) {
const params = {}
params.physicalnetworkid = physicalNetwork.id
api('listTrafficTypes', params).then(json => {
@@ -592,7 +592,7 @@ export default {
this.formPhysicalNetworkLoading = false
}
}).finally(() => {
- if (isLastNetwork) {
+ if (this.formPhysicalNetworks.length > 0 && this.isObjectEmpty(this.formSelectedPhysicalNetwork)) {
this.form.setFieldsValue({
physicalnetworkid: 0
})
@@ -736,6 +736,7 @@ export default {
message: this.$t('message.request.failed'),
description: this.$t('message.error.add.guest.network')
})
+ return
}
this.actionLoading = true
var params = {
```
----------------------------------------------------------------
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] davidjumani commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807935586
Might be a bit late to the party, but I think this can be solved better by appending the API result of `addPhysicalNetworkForGuestTrafficType` promise array, and then running a `Promise.all().then()`
Ref https://github.com/apache/cloudstack/blob/master/ui/src/components/view/SearchView.vue#L262
This way all the apis get resolved and finally in the `Promise.all().then()` the physicalnetwork can be set in the form. This takes care of any async issues that crop up
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-804767231
@shwstppr 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] davidjumani edited a comment on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807935586
Might be a bit late to the party, but I think this can be solved better by appending the API result of `addPhysicalNetworkForGuestTrafficType` to an array of promises, and then running a `Promise.all().then()` on the array
Ref https://github.com/apache/cloudstack/blob/master/ui/src/components/view/SearchView.vue#L262
This way all the apis get resolved and finally in the `Promise.all().then()` the physicalnetwork can be set in the form. This takes care of any async issues that crop up
--
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] ravening commented on a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r572723655
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -628,6 +628,14 @@ export default {
}
}
},
+ fetchFormPhysicalNetworkData () {
+ this.fetchPhysicalNetworkData()
+
+ // Wait until the data is fetched else it will result in NPE
+ while (this.formSelectedPhysicalNetwork === undefined ||
+ this.formSelectedPhysicalNetwork === null) {
+ }
Review comment:
@shwstppr actually the api `listPhysicalNetworks` never gets called and hence the value `this.formSelectedPhysicalNetwork` is not populated . Typically there should be atleast 1 physical network. so i dont much of an issue here
----------------------------------------------------------------
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] ravening commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-804744645
@shwstppr @rhtyd updated code
--
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] ravening commented on a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r587597691
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -592,11 +592,12 @@ export default {
this.formPhysicalNetworkLoading = false
}
}).finally(() => {
- if (isLastNetwork) {
+ if (this.formPhysicalNetworks.length > 0 && this.isObjectEmpty(this.formSelectedPhysicalNetwork)) {
this.form.setFieldsValue({
physicalnetworkid: 0
})
- this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
+ const network = (this.formPhysicalNetworks.length > 0) ? this.formPhysicalNetworks[0] : physicalNetwork
Review comment:
@weizhouapache no . try the change in commit caeaa1aadeea98bae1a14b733026f87c6f69dd63
if that works for you, i will merge the commits to 1
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-796554691
@ravening can you address review comments?
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-806406634
@shwstppr 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] rhtyd commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-786642584
Thanks @weizhouapache
Okay, I'll wait until this is ready for testing again @ravening
----------------------------------------------------------------
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] ravening commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-790716222
@weizhouapache @shwstppr I have pushed one commit. This change seems to work. can you guys test as well?
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-781531096
An issue also seems to crept in when creating guest networks from Infrastructure > Zones > Zone detail > Physical Networks > Physical Network > Traffic types > Guest
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-811698863
@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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-813243278
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4666 (SL-JID-87)
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807930400
UI build: :heavy_multiplication_x:
(SL-JID-75)
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-786455982
ping @ravening @shwstppr @weizhouapache what are the next steps?
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-813235612
@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] weizhouapache commented on a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r588201510
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -592,11 +592,12 @@ export default {
this.formPhysicalNetworkLoading = false
}
}).finally(() => {
- if (isLastNetwork) {
+ if (this.formPhysicalNetworks.length > 0 && this.isObjectEmpty(this.formSelectedPhysicalNetwork)) {
this.form.setFieldsValue({
physicalnetworkid: 0
})
- this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
+ const network = (this.formPhysicalNetworks.length > 0) ? this.formPhysicalNetworks[0] : physicalNetwork
Review comment:
@ravening it works without change on line 599-600.
a minor issue is, if I do not input the startip/endip/gw/netmask, I got two errors, 1 from UI, 1 from api.
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-805559385
@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] weizhouapache commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-781395444
I can reproduce the issue #4664 if there are two physical networks.
for example in settings below
physical network 1: MGT, contains traffic type: Guest/Management/Storage
physical network 2: PUB, contains traffic type: Public
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-809317605
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 275
--
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] davidjumani commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807929177
@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 a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r590158721
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -592,11 +592,12 @@ export default {
this.formPhysicalNetworkLoading = false
}
}).finally(() => {
- if (isLastNetwork) {
+ if (this.formPhysicalNetworks.length > 0 && this.isObjectEmpty(this.formSelectedPhysicalNetwork)) {
this.form.setFieldsValue({
physicalnetworkid: 0
})
- this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
+ const network = (this.formPhysicalNetworks.length > 0) ? this.formPhysicalNetworks[0] : physicalNetwork
Review comment:
@ravening Line 599-600 won't be needed with other changes in place.
For two different error messages issue can you please add `return` after line 738 (original file)
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-775791008
@ravening Also, if there is an issue with the form it should be fixed in 4.15.1
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-809279275
@ravening can you address review comments
@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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-801709352
Ping @ravening - can you address the review comments? Thanks.
----------------------------------------------------------------
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] ravening commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-776717356
> @ravening Also, if there is an issue with the form it should be fixed in 4.15.1
@shwstppr I have made some changes. please review again.
If the traffic type is not guest, then you are not pushing it into array and its calling another function with index 0. since array is empty, it will be null and hence we see this error
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-813888131
@davidjumani for further enhancement, feel free to raise a new PR
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-804747384
@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] rhtyd closed pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-806406118
@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 a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r572716532
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -628,6 +628,14 @@ export default {
}
}
},
+ fetchFormPhysicalNetworkData () {
+ this.fetchPhysicalNetworkData()
+
+ // Wait until the data is fetched else it will result in NPE
+ while (this.formSelectedPhysicalNetwork === undefined ||
+ this.formSelectedPhysicalNetwork === null) {
+ }
Review comment:
@ravening I've not been able to reproduce the original issue, were you able to identify why value remained null/undefined?
Also, in terms of the fix here should we have a timeout as it can lead to indefinite waiting if no value is found
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-811699407
@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] davidjumani edited a comment on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807935586
Might be a bit late to the party, but I think this can be solved better by appending the API result of `addPhysicalNetworkForGuestTrafficType` to an array of promises, and then running a `Promise.all().then()` on the array
Ref https://github.com/apache/cloudstack/blob/master/ui/src/components/view/SearchView.vue#L262
This way all the apis get resolved and finally in the `Promise.all().then()` the physicalnetwork can be set in the form. This takes care of any async issues that crop up
```
diff --git a/ui/src/views/network/CreateSharedNetworkForm.vue b/ui/src/views/network/CreateSharedNetworkForm.vue
index 9b020c18c5..a25f2604db 100644
--- a/ui/src/views/network/CreateSharedNetworkForm.vue
+++ b/ui/src/views/network/CreateSharedNetworkForm.vue
@@ -559,6 +559,7 @@ export default {
if (this.selectedZone === null || this.selectedZone === undefined) {
return
}
+ const promises = []
const params = {}
params.zoneid = this.selectedZone.id
this.formPhysicalNetworksLoading = true
@@ -566,38 +567,37 @@ export default {
this.formPhysicalNetworks = []
var networks = json.listphysicalnetworksresponse.physicalnetwork
if (this.arrayHasItems(networks)) {
- for (const i in networks) {
- this.addPhysicalNetworkForGuestTrafficType(networks[i], i * 1 === networks.length - 1)
+ for (const network of networks) {
+ promises.push(this.addPhysicalNetworkForGuestTrafficType(network))
}
- } else {
- this.formPhysicalNetworkLoading = false
}
- }).finally(() => {
+ })
+ Promise.all(promises).then(() => {
+ this.form.setFieldsValue({
+ physicalnetworkid: 0
+ })
+ this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
})
}
},
addPhysicalNetworkForGuestTrafficType (physicalNetwork, isLastNetwork) {
const params = {}
params.physicalnetworkid = physicalNetwork.id
- api('listTrafficTypes', params).then(json => {
- var trafficTypes = json.listtraffictypesresponse.traffictype
- if (this.arrayHasItems(trafficTypes)) {
- for (const i in trafficTypes) {
- if (trafficTypes[i].traffictype === 'Guest') {
- this.formPhysicalNetworks.push(physicalNetwork)
- break
+ return new Promise((resolve, reject) => {
+ api('listTrafficTypes', params).then(json => {
+ var trafficTypes = json.listtraffictypesresponse.traffictype
+ if (this.arrayHasItems(trafficTypes)) {
+ for (const type of trafficTypes) {
+ if (type.traffictype === 'Guest') {
+ this.formPhysicalNetworks.push(physicalNetwork)
+ break
+ }
}
}
- } else {
- this.formPhysicalNetworkLoading = false
- }
- }).finally(() => {
- if (isLastNetwork) {
- this.form.setFieldsValue({
- physicalnetworkid: 0
- })
- this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
- }
+ resolve()
+ }).catch(error => {
+ reject(error)
+ })
})
},
handlePhysicalNetworkChange (physicalNet) {
```
--
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-807929290
@davidjumani 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] weizhouapache commented on a change in pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#discussion_r587596130
##########
File path: ui/src/views/network/CreateSharedNetworkForm.vue
##########
@@ -592,11 +592,12 @@ export default {
this.formPhysicalNetworkLoading = false
}
}).finally(() => {
- if (isLastNetwork) {
+ if (this.formPhysicalNetworks.length > 0 && this.isObjectEmpty(this.formSelectedPhysicalNetwork)) {
this.form.setFieldsValue({
physicalnetworkid: 0
})
- this.handlePhysicalNetworkChange(this.formPhysicalNetworks[0])
+ const network = (this.formPhysicalNetworks.length > 0) ? this.formPhysicalNetworks[0] : physicalNetwork
Review comment:
@ravening do we still need the change in line 599 ?
----------------------------------------------------------------
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 #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-805559065
@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] weizhouapache commented on pull request #4666: Fix bug in creating shared network
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4666:
URL: https://github.com/apache/cloudstack/pull/4666#issuecomment-786522179
@rhtyd I have discussed with @ravening , this pr does not work.
in my testing, it works after removing the following lines
```
this.form.setFieldsValue({
physicalnetworkid: 0
})
```
----------------------------------------------------------------
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