[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3304: server: Fix exception while update domain resource count (#3204)

2019-04-30 Thread GitBox
DaanHoogland commented on a change in pull request #3304: server: Fix exception 
while update domain resource count (#3204)
URL: https://github.com/apache/cloudstack/pull/3304#discussion_r280008906
 
 

 ##
 File path: 
engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
 ##
 @@ -266,18 +266,18 @@ public long countMemoryAllocatedToAccount(long 
accountId) {
 }
 
 private long executeSqlCountComputingResourcesForAccount(long accountId, 
String sqlCountComputingResourcesAllocatedToAccount) {
-try (TransactionLegacy tx = TransactionLegacy.currentTxn()) {
+TransactionLegacy tx = TransactionLegacy.currentTxn()
 
 Review comment:
   fixed


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:
us...@infra.apache.org


With regards,
Apache Git Services


[cloudstack] branch reimplement3303on4.12 updated: missing ;

2019-04-30 Thread dahn
This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch reimplement3303on4.12
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/reimplement3303on4.12 by this 
push:
 new 4a93867  missing ;
4a93867 is described below

commit 4a9386726207bf6a374b1ee5bffad6e5c4129dbf
Author: dahn 
AuthorDate: Wed May 1 07:34:43 2019 +0200

missing ;
---
 .../src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
 
b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
index 9b6734d..edda682 100644
--- 
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
+++ 
b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
@@ -266,7 +266,7 @@ public class ResourceCountDaoImpl extends 
GenericDaoBase
 }
 
 private long executeSqlCountComputingResourcesForAccount(long accountId, 
String sqlCountComputingResourcesAllocatedToAccount) {
-TransactionLegacy tx = TransactionLegacy.currentTxn()
+TransactionLegacy tx = TransactionLegacy.currentTxn();
 try {
 PreparedStatement pstmt = 
tx.prepareAutoCloseStatement(sqlCountComputingResourcesAllocatedToAccount);
 pstmt.setLong(1, accountId);



[GitHub] [cloudstack] DaanHoogland commented on issue #3304: server: Fix exception while update domain resource count (#3204)

2019-04-30 Thread GitBox
DaanHoogland commented on issue #3304: server: Fix exception while update 
domain resource count (#3204)
URL: https://github.com/apache/cloudstack/pull/3304#issuecomment-488214271
 
 
   @rhtyd fwd-merge did not work due to the file being moved (out of git bounds)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] anuragaw commented on issue #3229: Console Proxy sends redundant CTRL and ALT keys to VMware

2019-04-30 Thread GitBox
anuragaw commented on issue #3229: Console Proxy sends redundant CTRL and ALT 
keys to VMware
URL: https://github.com/apache/cloudstack/issues/3229#issuecomment-488213157
 
 
   Also observed that `Cmd` (meta key on Macs) has no effect on VMWare and KVM 
when CPVM console is launched on MacBook Pro in Chrome.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] skattoju4 edited a comment on issue #3282: Fix slow vm creation when large sf snapshot count

2019-04-30 Thread GitBox
skattoju4 edited a comment on issue #3282: Fix slow vm creation when large sf 
snapshot count
URL: https://github.com/apache/cloudstack/pull/3282#issuecomment-488087008
 
 
   @svenvogel @syed as discussed / tested getUsedBytes is no longer 
contributing significantly to slowness in VM creation. Further investigation is 
needed to determine why VM creation from ISOs is sometimes slow. Further fixes 
can be addressed in separate PRs. If everyone agrees this one is ready be 
merged.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] PaulAngus commented on issue #3056: Niche feature configuration is always prominent in UI (SSL Offload)

2019-04-30 Thread GitBox
PaulAngus commented on issue #3056: Niche feature configuration is always 
prominent in UI (SSL Offload)
URL: https://github.com/apache/cloudstack/issues/3056#issuecomment-488099001
 
 
   It would certainly be nice.  I'm sure that the community would very much 
welcome the feature.  But for now the UI promises something that can't be 
provided most of the time.  And that's not ok...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] skattoju4 commented on issue #3282: Fix slow vm creation when large sf snapshot count

2019-04-30 Thread GitBox
skattoju4 commented on issue #3282: Fix slow vm creation when large sf snapshot 
count
URL: https://github.com/apache/cloudstack/pull/3282#issuecomment-488087008
 
 
   @svenvogel @syed as discussed / tested getUsedBytes is no longer 
contributing significantly to slowness in VM creation. Further investigation is 
needed to determine why VM creation from ISOs is sometimes slow. Further fixes 
can be addressed in separate PRs. If you guys agree this one is ready be merged.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] PaulAngus commented on a change in pull request #3258: [WIP DO NOT MERGE] Configurable UI branding, keyboard list and hide-able columns through a new config.js file

2019-04-30 Thread GitBox
PaulAngus commented on a change in pull request #3258:  [WIP DO NOT MERGE] 
Configurable UI branding, keyboard list and hide-able columns through a new 
config.js file
URL: https://github.com/apache/cloudstack/pull/3258#discussion_r279390920
 
 

 ##
 File path: ui/config.js
 ##
 @@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Define custom options configurable by admins for UI
+cloudStackOptions = {
+aboutText: "label.app.name",
 
 Review comment:
   ```suggestion
   aboutText: "label.app.name", // This is the Application 'Title' shown in 
the 'About' box
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] PaulAngus commented on a change in pull request #3258: [WIP DO NOT MERGE] Configurable UI branding, keyboard list and hide-able columns through a new config.js file

2019-04-30 Thread GitBox
PaulAngus commented on a change in pull request #3258:  [WIP DO NOT MERGE] 
Configurable UI branding, keyboard list and hide-able columns through a new 
config.js file
URL: https://github.com/apache/cloudstack/pull/3258#discussion_r279391480
 
 

 ##
 File path: ui/config.js
 ##
 @@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Define custom options configurable by admins for UI
+cloudStackOptions = {
+aboutText: "label.app.name",
+aboutTitle: "label.about.app",
+docTitle: "label.app.name",
 
 Review comment:
   ```suggestion
   docTitle: "label.app.name", // This is the Application 'Title' shown on 
browser tab and in 'About' box.
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] webermaximilian92 commented on issue #3307: Feature add scss to css compiler

2019-04-30 Thread GitBox
webermaximilian92 commented on issue #3307: Feature add scss to css compiler
URL: https://github.com/apache/cloudstack/pull/3307#issuecomment-487980076
 
 
   @rafaelweingartner 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] webermaximilian92 edited a comment on issue #3301: Add scss to css compiler

2019-04-30 Thread GitBox
webermaximilian92 edited a comment on issue #3301: Add scss to css compiler
URL: https://github.com/apache/cloudstack/pull/3301#issuecomment-487973640
 
 
   Very sorry I had to create new PR after running in problems with my forked 
cloudstack after switching my username.
   https://github.com/apache/cloudstack/pull/3307
   @rafaelweingartner 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] webermaximilian92 closed pull request #3301: Add scss to css compiler

2019-04-30 Thread GitBox
webermaximilian92 closed pull request #3301: Add scss to css compiler
URL: https://github.com/apache/cloudstack/pull/3301
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] webermaximilian92 commented on issue #3301: Add scss to css compiler

2019-04-30 Thread GitBox
webermaximilian92 commented on issue #3301: Add scss to css compiler
URL: https://github.com/apache/cloudstack/pull/3301#issuecomment-487973640
 
 
   Very sorry I had to create new PR after running in problems with my forked 
cloudstack after switching my username.
   https://github.com/apache/cloudstack/pull/3307


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] webermaximilian92 opened a new pull request #3307: Feature add scss to css compiler

2019-04-30 Thread GitBox
webermaximilian92 opened a new pull request #3307: Feature add scss to css 
compiler
URL: https://github.com/apache/cloudstack/pull/3307
 
 
   ## Description
   _Copy of https://github.com/apache/cloudstack/pull/3301 because I run into 
troubles after changing my username_
   
   
   This adds an gulp tasks, to build existing css files out of scss files and 
enables advantages for working with style rules. 
   Also this will reduce the complexibility of heavy css files **in all future 
css changes**.
   
   
   - Using CSS is no longer the common standard for complex pages like 
cloudstack
   - SASS (https://sass-lang.com/) or SCSS are preprocessors of CSS. 
Similar to ES6 or TypeScript for JavaScript.
   - SCSS files are compiled to CSS files ones while development or 
deployment, and not on every page request
   - SCSS provides us many logical functions and advantages to organise CSS 
rules more easily and less redundand with features like variables and so called 
mixins
   - SCSS let us work more modular and is better maintainable
   - We could split CSS files too, but this is not best practice, since 
many CSS files will result in many server requests and with SCSS it's possible 
to compile many files into one single CSS file (full automatically in just 
under a second after any SCSS changes)
   - This compiled CSS file could also get minified, which will enhance 
loading times of cloudstack pages.
   
   
   
   
   
   
   
   
   
   ## Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [x] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [x] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   
   ## How Has This Been Tested?
   
   (This changes modify and unify the structure of css code only.)
   All we have to check yet, is if .css file-changes include only: indentation, 
spaces and quotes.
   
   It's very useful to reduce the diff to non whitespace changes here:
   ![Screenshot 2019-04-26 at 16 23 
44](https://user-images.githubusercontent.com/46194969/56814734-14f05080-6840-11e9-9924-23559ae6dd1f.png)
   
   
   
   
   
   
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] ustcweizhou commented on a change in pull request #3227: ubuntu16: fix three issues with ubuntu 16.04 hosts

2019-04-30 Thread GitBox
ustcweizhou commented on a change in pull request #3227: ubuntu16:  fix three 
issues with ubuntu 16.04 hosts
URL: https://github.com/apache/cloudstack/pull/3227#discussion_r279762470
 
 

 ##
 File path: 
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
 ##
 @@ -914,6 +915,20 @@ public boolean configure(final String name, final 
Map params) th
 throw new CloudRuntimeException(e.getMessage());
 }
 
+// destroy default network, see 
https://libvirt.org/sources/java/javadoc/org/libvirt/Network.html
+try {
+Network network = conn.networkLookupByName("default");
+s_logger.debug("Found libvirt default network, destroying it and 
setting autostart to false");
+if (network.isActive() == 1) {
+network.destroy();
+}
+if (network.getAutostart()) {
 
 Review comment:
   @rhtyd 
   I checked the code again, the packaging/debian/cloudstack-agent.init will be 
packaged in ubuntu/debs and deployed on ubuntu servers as 
/etc/init.d/cloudstack-agent. 
   Since we do not support ubuntu 12.04 any more , and systemd is used in 
ubuntu 14.04 and later, I think this change will not break other functions.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] ustcweizhou commented on issue #3178: GUI project pulldown list slowdown on large number of projects

2019-04-30 Thread GitBox
ustcweizhou commented on issue #3178: GUI project pulldown list slowdown on 
large number of projects
URL: https://github.com/apache/cloudstack/issues/3178#issuecomment-487940581
 
 
   @svenvogel @rhtyd @DagSonsteboSB 
   PR has been created for this issue: 
https://github.com/apache/cloudstack/pull/3306
   
   We have similar issue while listing vms, I will create another pull request 
when #3306 is merged because it requires db change 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] ustcweizhou opened a new pull request #3306: server: reduce execution time while listing project if projects have many resource tags

2019-04-30 Thread GitBox
ustcweizhou opened a new pull request #3306: server: reduce execution time 
while listing project if projects have many resource tags
URL: https://github.com/apache/cloudstack/pull/3306
 
 
   
   ## Description
   
   If projects have many resource tags, it will take long time to list projects.
   Remove resource tags information from project_view will fix it the issue.
   
   Fixes: #3178
   
   ## 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)
   
   
   ## How Has This Been Tested?
   
   
   
   
   create project,
   add resource tags to project,
   listprojects  lists all resouce tags.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] ustcweizhou commented on issue #3296: [4.13] noVNC console

2019-04-30 Thread GitBox
ustcweizhou commented on issue #3296: [4.13] noVNC console
URL: https://github.com/apache/cloudstack/pull/3296#issuecomment-487935007
 
 
   @DennisKonrad 
   python-numpy is required in CPVM. It would be nice to have it pre-installed 
in systemvm template. However, if CPVM has internet connection it is not a 
problem, because python-numpy will be installed when noVNC is launched in CPVM.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] rafaelweingartner commented on issue #3301: Add scss to css compiler

2019-04-30 Thread GitBox
rafaelweingartner commented on issue #3301: Add scss to css compiler
URL: https://github.com/apache/cloudstack/pull/3301#issuecomment-487917743
 
 
   Thanks @webermaximilian92.
   Can you check the licensing issues?
   
https://builds.apache.org/job/cloudstack-pr-analysis/8442/artifact/target/rat.txt


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] DennisKonrad commented on issue #3190: Include 'removed' async jobs to check recurring snapshots

2019-04-30 Thread GitBox
DennisKonrad commented on issue #3190: Include 'removed' async jobs to check 
recurring snapshots
URL: https://github.com/apache/cloudstack/pull/3190#issuecomment-487914674
 
 
   @rhtyd Hi, we tested this PR thoroughly. I think it's ready to merge 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] svenvogel commented on issue #3294: Set default service offering over api and ui

2019-04-30 Thread GitBox
svenvogel commented on issue #3294: Set default service offering over api and ui
URL: https://github.com/apache/cloudstack/pull/3294#issuecomment-487914592
 
 
   @DaanHoogland can you approve?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] svenvogel commented on issue #3199: Fix ip and ip cidr column sorting in tables

2019-04-30 Thread GitBox
svenvogel commented on issue #3199: Fix ip and ip cidr column sorting in tables
URL: https://github.com/apache/cloudstack/pull/3199#issuecomment-487913223
 
 
   @GabrielBrascher @borisstoyanov can you approve that?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] DennisKonrad commented on issue #3296: [4.13] noVNC console

2019-04-30 Thread GitBox
DennisKonrad commented on issue #3296: [4.13] noVNC console
URL: https://github.com/apache/cloudstack/pull/3296#issuecomment-487907424
 
 
   @ustcweizhou To migrate from an existing installation, do we need to build a 
new systemvm template? Or are the changes to the CPVM applied online somehow?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] anuragaw commented on issue #3229: Console Proxy sends redundant CTRL and ALT keys to VMware

2019-04-30 Thread GitBox
anuragaw commented on issue #3229: Console Proxy sends redundant CTRL and ALT 
keys to VMware
URL: https://github.com/apache/cloudstack/issues/3229#issuecomment-487894349
 
 
   Upon investigation it seems to be broken just for VMWare
   
   > Seems to be working fine on XenServer (7.0 tested) - I can see with 
"showkey" only one keycode reported to be pressed/released. Didn't try KVM.
   
   Seems to be working fine on KVM on latest Master. Investigating further.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] ustcweizhou commented on a change in pull request #3304: server: Fix exception while update domain resource count (#3204)

2019-04-30 Thread GitBox
ustcweizhou commented on a change in pull request #3304: server: Fix exception 
while update domain resource count (#3204)
URL: https://github.com/apache/cloudstack/pull/3304#discussion_r279634207
 
 

 ##
 File path: 
engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
 ##
 @@ -266,18 +266,18 @@ public long countMemoryAllocatedToAccount(long 
accountId) {
 }
 
 private long executeSqlCountComputingResourcesForAccount(long accountId, 
String sqlCountComputingResourcesAllocatedToAccount) {
-try (TransactionLegacy tx = TransactionLegacy.currentTxn()) {
+TransactionLegacy tx = TransactionLegacy.currentTxn()
 
 Review comment:
   @DaanHoogland  you missed ";" at the end of line 269.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [cloudstack] kioie commented on issue #3293: Limit selecting of secondary storage with 90% utilization

2019-04-30 Thread GitBox
kioie commented on issue #3293: Limit selecting of secondary storage with 90% 
utilization
URL: https://github.com/apache/cloudstack/pull/3293#issuecomment-487833694
 
 
   > Simply returning `null` could possibly result in newer bugs. ( On a quick 
inspection it seems that the method consumers would not always check for null 
values ).
   > 
   > I wonder if more testing scenarios than mentioned on the PR should be 
covered. Thoughts?
   
   I debated with the thought of editing the method consumers to handle null 
returns intrinsically, but I noticed in most cases (at least to the best of my 
knowledge) null returns are eventually handled by level 2 consumers. I'd be 
curious to try out other related test cases


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:
us...@infra.apache.org


With regards,
Apache Git Services