[GitHub] blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs

[GitBox]

blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT 
configuration and VMs not accessing each other via public IPs
URL: https://github.com/apache/cloudstack/pull/2514#issuecomment-382146230
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1939


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs

[GitBox]

blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT 
configuration and VMs not accessing each other via public IPs
URL: https://github.com/apache/cloudstack/pull/2514#issuecomment-382134856
 
 
   @rafaelweingartner 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 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] rafaelweingartner commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-382061698
 
 
   @nitin-maharana thank you for the review ;)
   Thanks!


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] nitin-maharana commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-382061346
 
 
   Thanks, @rafaelweingartner for making the changes, I will review rest of the 
changes by tomorrow or day after.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts

[GitBox]

blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is 
missing while prioritizing hosts
URL: https://github.com/apache/cloudstack/pull/2577#issuecomment-382055913
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1938


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] khos2ow commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts

[GitBox]

khos2ow commented on a change in pull request #2577: Prevent NPE if guest OS 
mapping is missing while prioritizing hosts
URL: https://github.com/apache/cloudstack/pull/2577#discussion_r182139455
 
 

 ##
 File path: 
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
 ##
 @@ -417,6 +419,10 @@ public boolean isVirtualMachineUpgradable(VirtualMachine 
vm, ServiceOffering off
 // Determine the guest OS category of the template
 String templateGuestOSCategory = getTemplateGuestOSCategory(template);
 
+if (Strings.isNullOrEmpty(templateGuestOSCategory)) {
 
 Review comment:
   Technically `templateGuestOSCategory == null` would be enough, and I cannot 
think of a way `getTemplateGuestOSCategory` returning empty string unless the 
database record integrity has been changed, that's why I used 
`Strings.isNullOrEmpty` rather rather than a simple `== null` check. (which 
might be slightly overkill at the same time!)


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts

[GitBox]

rafaelweingartner commented on a change in pull request #2577: Prevent NPE if 
guest OS mapping is missing while prioritizing hosts
URL: https://github.com/apache/cloudstack/pull/2577#discussion_r182134818
 
 

 ##
 File path: 
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
 ##
 @@ -417,6 +419,10 @@ public boolean isVirtualMachineUpgradable(VirtualMachine 
vm, ServiceOffering off
 // Determine the guest OS category of the template
 String templateGuestOSCategory = getTemplateGuestOSCategory(template);
 
+if (Strings.isNullOrEmpty(templateGuestOSCategory)) {
 
 Review comment:
   if you return a `null` value in `getTemplateGuestOSCategory` method, why not 
simply check `templateGuestOSCategory == null`?
   
   Is it possible to have an empty or blank `guestOSCategory.name`? 
   BTW: `Strings.isNullOrEmpty` will return `false` for blanks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts

[GitBox]

blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is 
missing while prioritizing hosts
URL: https://github.com/apache/cloudstack/pull/2577#issuecomment-382046711
 
 
   @khos2ow 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 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] khos2ow opened a new pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts

[GitBox]

khos2ow opened a new pull request #2577: Prevent NPE if guest OS mapping is 
missing while prioritizing hosts
URL: https://github.com/apache/cloudstack/pull/2577
 
 
   ## Description
   
   When starting an existing VM and trying to prioritize hosts, if Guest OS 
mapping has been removed from that host, there will be a NullPointerException 
which prevents the VM to be started successfully.
   
   
   
   
   ## 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)
   
   ## GitHub Issue/PRs
   
   
   
   
   
   
   ## Screenshots (if appropriate):
   
   ## How Has This Been Tested?
   
   
   
   
   
   ## Checklist:
   
   
   - [ ] I have read the 
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md)
 document.
   - [ ] My code follows the code style of this project.
   - [ ] My change requires a change to the documentation.
   - [ ] I have updated the documentation accordingly.
   Testing
   - [ ] I have added tests to cover my changes.
   - [ ] All relevant new and existing integration tests have passed.
   - [ ] A full integration testsuite with all test that can run on my 
environment has passed.
   
   
   @blueorangutan package
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2573: Cloudstack 10356

[GitBox]

rafaelweingartner commented on a change in pull request #2573: Cloudstack 10356
URL: https://github.com/apache/cloudstack/pull/2573#discussion_r182091684
 
 

 ##
 File path: 
server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
 ##
 @@ -740,14 +740,14 @@ public boolean startRemoteAccessVpn(final 
RemoteAccessVpn vpn, final VirtualRout
 throw new AgentUnavailableException("Unable to send commands to 
virtual router ", router.getHostId(), e);
 }
 Answer answer = cmds.getAnswer("users");
-if (!answer.getResult()) {
+if (answer != null && !answer.getResult()) {
 
 Review comment:
   Sorry, I did not understand your question. Are you asking if I would suggest 
to use BooleanUtils here as well? Please, go ahead and do that.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] lujiefsi commented on a change in pull request #2573: Cloudstack 10356

[GitBox]

lujiefsi commented on a change in pull request #2573: Cloudstack 10356
URL: https://github.com/apache/cloudstack/pull/2573#discussion_r182090707
 
 

 ##
 File path: 
server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
 ##
 @@ -740,14 +740,14 @@ public boolean startRemoteAccessVpn(final 
RemoteAccessVpn vpn, final VirtualRout
 throw new AgentUnavailableException("Unable to send commands to 
virtual router ", router.getHostId(), e);
 }
 Answer answer = cmds.getAnswer("users");
-if (!answer.getResult()) {
+if (answer != null && !answer.getResult()) {
 
 Review comment:
   hum, here is not change as @rafaelweingartner suggest


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381998091
 
 
   @rhtyd 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 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381997838
 
 
   @blueorangutan test 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS)

[GitBox]

rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI 
to 1.11 (JQuery UI 1.8.4 prone to XSS)
URL: https://github.com/apache/cloudstack/pull/2524#issuecomment-381990262
 
 
   Thanks @GabrielBrascher! I fixed both of the issues you raised.
   What do you guys think? Should I change the non-minified version of 
Jquery-Ui to the minified one?
   
   Others (@khos2ow, @rhtyd, @DaanHoogland and so on), could you also help here?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381988851
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1937


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381979907
 
 
   @rafaelweingartner 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 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] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute 
by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381978823
 
 
   @blueorangutan package.
   
   @rhtyd I think everything is fine now.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on a change in pull request #2574: 
[CLOUDSTACK-5235] ask users old password when they are executing a password 
update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051964
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
+if (password == null) {
+s_logger.trace("No new password to update for user: " + 
user.getUuid());
+return;
 }
-
-// don't allow updating system account
-if (account.getId() == Account.ACCOUNT_ID_SYSTEM) {
-throw new PermissionDeniedException("user id : " + userId + " is 
system account, update is not allowed");
+if (StringUtils.isBlank(password)) {
+throw new InvalidParameterValueException("Password cannot be empty 
or blank.");
+}
+Account 

[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on a change in pull request #2574: 
[CLOUDSTACK-5235] ask users old password when they are executing a password 
update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051870
 
 

 ##
 File path: ui/scripts/accounts.js
 ##
 @@ -1476,6 +1476,14 @@
 form: {
 title: 
'label.action.change.password',
 fields: {
+oldPassword: {
+label: 
'label.old.password',
+isPassword: true,
+validation: {
+required: 
!(isAdmin() || isDomainAdmin())
 
 Review comment:
   I thought about that, but I was under the impression that the field would 
still be validated by the javascript framework we have. I could not find a 
`hide` parameter. Therefore, I am hiding it manually.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on a change in pull request #2574: 
[CLOUDSTACK-5235] ask users old password when they are executing a password 
update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051594
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
 
 Review comment:
   done


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] 
ask users old password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182041837
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
+if (password == null) {
+s_logger.trace("No new password to update for user: " + 
user.getUuid());
+return;
 }
-
-// don't allow updating system account
-if (account.getId() == Account.ACCOUNT_ID_SYSTEM) {
-throw new PermissionDeniedException("user id : " + userId + " is 
system account, update is not allowed");
+if (StringUtils.isBlank(password)) {
+throw new InvalidParameterValueException("Password cannot be empty 
or blank.");
+}
+Account 

[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] 
ask users old password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182039920
 
 

 ##
 File path: ui/scripts/accounts.js
 ##
 @@ -1476,6 +1476,14 @@
 form: {
 title: 
'label.action.change.password',
 fields: {
+oldPassword: {
+label: 
'label.old.password',
+isPassword: true,
+validation: {
+required: 
!(isAdmin() || isDomainAdmin())
 
 Review comment:
   As we are hiding this field for admin and domain admin. I think we can 
assign the value true for the required parameter.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] 
ask users old password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182038769
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
 
 Review comment:
   I mean the first parameter (password -> newPassword)


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955944
 
 
   let me kick tests
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381956264
 
 
   Nevermind, @rafaelweingartner please kick packaging job with BO once you're 
able to re-fix the styles issues. Sorry for the additional trouble on porting 
to 4.11.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on a change in pull request #2576: Fix Python code 
checkstyle execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182038641
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   That is what I expected to happen, but it seems it did not work.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955944
 
 
   let me kick tests
   @blueorangutan package


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute 
by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955657
 
 
   Well, now, the error in Travis are related to the code style. I will apply 
the code style fixes too then.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182038138
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   If dependencies are already installed, it should not cause an issue. The 
`--upgrade` should upgrade an installed package and its dependencies, but 
nonethless let's see how the Travis run goes.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on a change in pull request #2574: 
[CLOUDSTACK-5235] ask users old password when they are executing a password 
update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182035843
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
 
 Review comment:
   Do you mean `validateUserNewPasswordAndUpdateIfNeeded`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

rafaelweingartner commented on a change in pull request #2574: 
[CLOUDSTACK-5235] ask users old password when they are executing a password 
update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182035508
 
 

 ##
 File path: api/src/main/java/com/cloud/user/AccountService.java
 ##
 @@ -23,53 +23,28 @@
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
 import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
+import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
 
 import com.cloud.domain.Domain;
 import com.cloud.exception.PermissionDeniedException;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.ServiceOffering;
 
-
 public interface AccountService {
 
 /**
  * Creates a new user and account, stores the password as is so encrypted 
passwords are recommended.
- *
 
 Review comment:
   Well, I do like Java documentation; however, in this case, the use of 
`@param` does not bring benefits. I mean, if each one of the parameters was 
fully described it would be ok, but that is not what happened here. That is why 
I removed them.
   
   I am sorry Nitin, but I would rather not document those parameters. Instead, 
we should get rid of these methods with a lot of parameters. My changes affect 
only the `updateUser` method (I already refactored that whole method). 
Afterwards, I executed a cleanup in the classes that I had modified some code.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute 
by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381950233
 
 
   @rhtyd rebased against 4.11. I also added only the dependencies upgrade. 
Let's see what happens.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on a change in pull request #2576: Fix Python code 
checkstyle execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r18205
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   I think the problems happens when the dependencies are already installed. 
Then, it seems that for some cases `pip` does not upgrade them if you do not 
ask to. 
   
   Again, I am not a Python guy, so that that hypothesis with a grain of salt.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381948034
 
 
   I'll kick test once the PR is re-targeted for 4.11 branch @rafaelweingartner 


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182032036
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   Another note, the new release of pip (v10) might deprecate few things -- I 
know python 2.6.x is not well supported now.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182031926
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   @rafaelweingartner ideally, pip install should install additional 
dependencies we should not need to install dependencies for pylint manually.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182031799
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 Review comment:
   Thanks, np I was just curious why we were doing it that way. (love the 
expression of the smiley btw)


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute 
by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381945912
 
 
   @rhtyd Locally my tests were also passing :(...
   
   Actually, locally I received those problems regarding code formatting. That 
is why I fixed them too. However, even after fixing all of them, Travis would 
not work. Then, I started digging deeper in the dependencies and looking for 
incompatibilities (this is a nightmare!).


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] 
ask users old password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182030109
 
 

 ##
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##
 @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String 
password, String firstName, Str
 
 @Override
 @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = 
"creating User")
-public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId,
-String userUUID) {
+public UserVO createUser(String userName, String password, String 
firstName, String lastName, String email, String timeZone, String accountName, 
Long domainId, String userUUID) {
 
 return createUser(userName, password, firstName, lastName, email, 
timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN);
 }
 
 @Override
-@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"updating User")
-public UserAccount updateUser(Long userId, String firstName, String 
lastName, String email, String userName, String password, String apiKey, String 
secretKey,
-String timeZone) {
-// Input validation
-UserVO user = _userDao.getUser(userId);
+@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = 
"Updating User")
+public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
+UserVO user = retrieveAndValidateUser(updateUserCmd);
+s_logger.debug("Updating user with Id: " + user.getUuid());
 
-if (user == null) {
-throw new InvalidParameterValueException("unable to find user by 
id");
-}
+validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user);
+Account account = retrieveAndValidateAccount(user);
 
-if ((apiKey == null && secretKey != null) || (apiKey != null && 
secretKey == null)) {
-throw new InvalidParameterValueException("Please provide an 
userApiKey/userSecretKey pair");
-}
+validateAndUpdateFirstNameIfNeeded(updateUserCmd, user);
+validateAndUpdateLastNameIfNeeded(updateUserCmd, user);
+validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account);
 
-// If the account is an admin type, return an error. We do not allow 
this
-Account account = _accountDao.findById(user.getAccountId());
-if (account == null) {
-throw new InvalidParameterValueException("unable to find user 
account " + user.getAccountId());
+validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), 
user, updateUserCmd.getOldPassword());
+String email = updateUserCmd.getEmail();
+if (StringUtils.isNotBlank(email)) {
+user.setEmail(email);
 }
-
-// don't allow updating project account
-if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
-throw new InvalidParameterValueException("unable to find user by 
id");
+String timezone = updateUserCmd.getTimezone();
+if (StringUtils.isNotBlank(timezone)) {
+user.setTimezone(timezone);
+}
+_userDao.update(user.getId(), user);
+return _userAccountDao.findById(user.getId());
+}
+
+/**
+ * Updates the password in the user POJO if needed. If no password is 
provided, then the password is not updated.
+ * The following validations are executed if 'password' is not null. 
Admins (root admins or domain admins) can execute password updates without 
entering the old password.
+ * 
+ *   If 'password' is blank, we throw an {@link 
InvalidParameterValueException};
+ *   If old password is not provided and user is not an Admin, we 
throw an {@link InvalidParameterValueException};
+ *   If a normal user is calling this method, we use {@link 
#validateOldPassword(UserVO, String)} to check if the provided old password 
matches the database one;
+ * 
+ *
+ * If all checks pass, we encode the given password with the most 
preferable password mechanism given in {@link #_userPasswordEncoders}.
+ */
+protected void validateUserPasswordAndUpdateIfNeeded(String password, 
UserVO user, String oldPassword) {
 
 Review comment:
   I think newPassword name would be more meaningful.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on a change in pull request #2576: Fix Python code 
checkstyle execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182029928
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   `Markdown` I added because I was getting a message saying that it is not 
installed, but some of these dependencies might require it. That is why I 
installed.
   
   `six and astroid` seems to be used by `pylint` and `nose`. And I think that 
is the problem we were having. There is an issue regarding these dependencies 
versions, which are not automatically upgraded when you install some library 
the relies on them.
   
   `¯\_(ツ)_/¯`


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on a change in pull request #2576: Fix Python code 
checkstyle execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182029173
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 Review comment:
   ¯\_(ツ)_/¯...
   I really do not understand this. Most of the results I found on Google were 
telling me to remove and install again the library. I guess when people do not 
know what they are doing, they do this things.
   
   Well, I will remove and see what happens.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] 
ask users old password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182028887
 
 

 ##
 File path: api/src/main/java/com/cloud/user/AccountService.java
 ##
 @@ -23,53 +23,28 @@
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd;
 import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
+import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
 
 import com.cloud.domain.Domain;
 import com.cloud.exception.PermissionDeniedException;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.ServiceOffering;
 
-
 public interface AccountService {
 
 /**
  * Creates a new user and account, stores the password as is so encrypted 
passwords are recommended.
- *
 
 Review comment:
   @rafaelweingartner, Thanks for the fix. I think it's better to keep the 
comments, would be helpful in the documentation. Would be great if you describe 
a bit about each parameter.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rafaelweingartner commented on a change in pull request #2576: Fix Python code 
checkstyle execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182028493
 
 

 ##
 File path: systemvm/test/runtests.sh
 ##
 @@ -21,9 +21,9 @@
 export PYTHONPATH="../debian/opt/cloud/bin/"
 export PYTHONDONTWRITEBYTECODE=False
 
-echo "Running pep8 to check systemvm/python code for errors"
-pep8 --max-line-length=179 *py
-pep8 --max-line-length=179 
--exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py `find 
../debian -name \*.py`
+echo "Running pycodestyle to check systemvm/python code for errors"
+pycodestyle --max-line-length=179 *py
 
 Review comment:
   I was getting a message that `pep8` is deprecated and is being replaced by 
`pycodestyle`, that is why I replaced. It was one of my commits trying to solve 
the problem (or at least change some error message).


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381895005
 
 
   @borisstoyanov 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 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381894866
 
 
   @borisstoyanov 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 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] blueorangutan commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

blueorangutan commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM 
Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-381894661
 
 
   @borisstoyanov 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 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] borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381894577
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian

[GitBox]

blueorangutan commented on issue #2550: debian: Use only `-l` for libvirtd 
default file on debian
URL: https://github.com/apache/cloudstack/pull/2550#issuecomment-381894539
 
 
   @borisstoyanov a Trillian-Jenkins test job (ubuntu mgmt + kvm-ubuntu) 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 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] borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381894511
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2376: [4.11] Smoketest Health Check

[GitBox]

blueorangutan commented on issue #2376: [4.11] Smoketest Health Check
URL: https://github.com/apache/cloudstack/pull/2376#issuecomment-381894276
 
 
   @borisstoyanov a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 
mgmt + vmware65, centos7 mgmt + kvmcentos7) 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 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] borisstoyanov commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian

[GitBox]

borisstoyanov commented on issue #2550: debian: Use only `-l` for libvirtd 
default file on debian
URL: https://github.com/apache/cloudstack/pull/2550#issuecomment-381894393
 
 
   @blueorangutan test ubuntu kvm-ubuntu


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] borisstoyanov commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM

[GitBox]

borisstoyanov commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM 
Migration for KVM
URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-381894193
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] borisstoyanov commented on issue #2376: [4.11] Smoketest Health Check

[GitBox]

borisstoyanov commented on issue #2376: [4.11] Smoketest Health Check
URL: https://github.com/apache/cloudstack/pull/2376#issuecomment-381893956
 
 
   @blueorangutan test matrix


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381882865
 
 
   @borisstoyanov 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 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] borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update

[GitBox]

borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old 
password when they are executing a password update
URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381882481
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381882211
 
 
   @borisstoyanov 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 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] borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381882063
 
 
   @blueorangutan test


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381879043
 
 
   Thanks @rafaelweingartner I see the issue is Travis related, thanks for 
fixing. I've left some remarks. Also, the Travis failure affects 4.11 branch 
too, can you rebase and edit this PR against 4.11?
   Locally unable to reproduce the issue;
   On 4.11, I get this:
   ```
   > bash -x runtests.sh 
   + export PYTHONPATH=../debian/opt/cloud/bin/
   + PYTHONPATH=../debian/opt/cloud/bin/
   + export PYTHONDONTWRITEBYTECODE=False
   + PYTHONDONTWRITEBYTECODE=False
   + echo 'Running pep8 to check systemvm/python code for errors'
   Running pep8 to check systemvm/python code for errors
   + pep8 --max-line-length=179 TestCsAddress.py TestCsApp.py TestCsCmdLine.py 
TestCsConfig.py TestCsDatabag.py TestCsDhcp.py TestCsFile.py 
TestCsGuestNetwork.py TestCsHelper.py TestCsInterface.py TestCsNetfilter.py 
TestCsProcess.py TestCsRedundant.py TestCsRoute.py TestCsRule.py
   ++ find ../debian -name '*.py'
   + pep8 --max-line-length=179 
--exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py 
../debian/opt/cloud/bin/cs_loadbalancer.py ../debian/opt/cloud/bin/configure.py 
../debian/opt/cloud/bin/master.py ../debian/opt/cloud/bin/cs_ip.py 
../debian/opt/cloud/bin/update_config.py 
../debian/opt/cloud/bin/cs_guestnetwork.py 
../debian/opt/cloud/bin/passwd_server_ip.py 
../debian/opt/cloud/bin/cs_staticroutes.py ../debian/opt/cloud/bin/cs_dhcp.py 
../debian/opt/cloud/bin/cs_vpnusers.py ../debian/opt/cloud/bin/set_redundant.py 
../debian/opt/cloud/bin/cs_remoteaccessvpn.py 
../debian/opt/cloud/bin/baremetal-vr.py 
../debian/opt/cloud/bin/cs_monitorservice.py 
../debian/opt/cloud/bin/cs_site2sitevpn.py ../debian/opt/cloud/bin/merge.py 
../debian/opt/cloud/bin/cs_vmp.py ../debian/opt/cloud/bin/cs_forwardingrules.py 
../debian/opt/cloud/bin/cs_vmdata.py ../debian/opt/cloud/bin/vmdata.py 
../debian/opt/cloud/bin/cs/CsStaticRoutes.py 
../debian/opt/cloud/bin/cs/CsFile.py ../debian/opt/cloud/bin/cs/CsDhcp.py 
../debian/opt/cloud/bin/cs/CsDatabag.py ../debian/opt/cloud/bin/cs/CsProcess.py 
../debian/opt/cloud/bin/cs/CsRoute.py ../debian/opt/cloud/bin/cs/CsAddress.py 
../debian/opt/cloud/bin/cs/CsMonitor.py 
../debian/opt/cloud/bin/cs/CsLoadBalancer.py 
../debian/opt/cloud/bin/cs/__init__.py 
../debian/opt/cloud/bin/cs/CsNetfilter.py 
../debian/opt/cloud/bin/cs/CsGuestNetwork.py 
../debian/opt/cloud/bin/cs/CsConfig.py ../debian/opt/cloud/bin/cs/CsRule.py 
../debian/opt/cloud/bin/cs/CsRedundant.py ../debian/opt/cloud/bin/cs/CsApp.py 
../debian/opt/cloud/bin/cs/CsHelper.py ../debian/opt/cloud/bin/line_edit.py 
../debian/opt/cloud/bin/cs_cmdline.py 
../debian/opt/cloud/bin/cs_firewallrules.py 
../debian/opt/cloud/bin/cs_network_acl.py ../debian/root/monitorServices.py
   + '[' 0 -gt 0 ']'
   + echo 'Running pylint to check systemvm/python code for errors'
   Running pylint to check systemvm/python code for errors
   + pylint --disable=R,C,W TestCsAddress.py TestCsApp.py TestCsCmdLine.py 
TestCsConfig.py TestCsDatabag.py TestCsDhcp.py TestCsFile.py 
TestCsGuestNetwork.py TestCsHelper.py TestCsInterface.py TestCsNetfilter.py 
TestCsProcess.py TestCsRedundant.py TestCsRoute.py TestCsRule.py
   No config file found, using default configuration
   
   
   Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)
   
   ++ find ../debian -name '*.py'
   + pylint --disable=R,C,W ../debian/opt/cloud/bin/cs_loadbalancer.py 
../debian/opt/cloud/bin/configure.py ../debian/opt/cloud/bin/master.py 
../debian/opt/cloud/bin/cs_ip.py ../debian/opt/cloud/bin/update_config.py 
../debian/opt/cloud/bin/cs_guestnetwork.py 
../debian/opt/cloud/bin/passwd_server_ip.py 
../debian/opt/cloud/bin/cs_staticroutes.py ../debian/opt/cloud/bin/cs_dhcp.py 
../debian/opt/cloud/bin/cs_vpnusers.py ../debian/opt/cloud/bin/set_redundant.py 
../debian/opt/cloud/bin/cs_remoteaccessvpn.py 
../debian/opt/cloud/bin/baremetal-vr.py 
../debian/opt/cloud/bin/cs_monitorservice.py 
../debian/opt/cloud/bin/cs_site2sitevpn.py ../debian/opt/cloud/bin/merge.py 
../debian/opt/cloud/bin/cs_vmp.py ../debian/opt/cloud/bin/cs_forwardingrules.py 
../debian/opt/cloud/bin/cs_vmdata.py ../debian/opt/cloud/bin/vmdata.py 
../debian/opt/cloud/bin/cs/CsStaticRoutes.py 
../debian/opt/cloud/bin/cs/CsFile.py ../debian/opt/cloud/bin/cs/CsDhcp.py 
../debian/opt/cloud/bin/cs/CsDatabag.py ../debian/opt/cloud/bin/cs/CsProcess.py 
../debian/opt/cloud/bin/cs/CsRoute.py ../debian/opt/cloud/bin/cs/CsAddress.py 
../debian/opt/cloud/bin/cs/CsMonitor.py 
../debian/opt/cloud/bin/cs/CsLoadBalancer.py 
../debian/opt/cloud/bin/cs/__init__.py 
../debian/opt/cloud/bin/cs/CsNetfilter.py 
../debian/opt/cloud/bin/cs/CsGuestNetwork.py 
../debian/opt/cloud/bin/cs/CsConfig.py ../debian/opt/cloud/bin/cs/CsRule.py 
../debian/opt/cloud/bin/cs/CsRedundant.py 

[cloudstack] branch master updated (c43c69a -> b940a89)

[rohit]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.


from c43c69a  Merge release branch 4.11 to master
 add 392f62d  consoleproxy: use consoleproxy.domain for non-ssl enable env 
(#2562)
 new b940a89  Merge branch '4.11'

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 core/src/main/java/com/cloud/info/ConsoleProxyInfo.java  | 5 +
 .../main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java| 5 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
ro...@apache.org.

[cloudstack] 01/01: Merge branch '4.11'

[rohit]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit b940a892f76c32b761dc7a8eec0f9a3197c5ba30
Merge: c43c69a 392f62d
Author: Rohit Yadav 
AuthorDate: Tue Apr 17 12:58:39 2018 +0530

Merge branch '4.11'

 core/src/main/java/com/cloud/info/ConsoleProxyInfo.java  | 5 +
 .../main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java| 5 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)


-- 
To stop receiving notification emails like this one, please contact
ro...@apache.org.

[GitHub] MartinEmrich commented on issue #2561: cloud-early-config detects unknown hypervisor type "xen-domU"

[GitBox]

MartinEmrich commented on issue #2561: cloud-early-config detects unknown 
hypervisor type "xen-domU"
URL: https://github.com/apache/cloudstack/issues/2561#issuecomment-381661246
 
 
   I wiped everything and started over: Install 4.9.2.0 with some test VMs, 
Upgrade to 4.11.0-SNAPSHOT with the systemvm template from @rhtyd. I registered 
the System VM template
   
   * as Debian 7 (4.9.2 does not offer something newer)
   * with no checkboxes checked except "HVM" (the default selection).
   
   After clicking "Update", the new systemVMs this time come up as 
paravirtualized (despite the checkbox "HVM"). The initialization took more than 
half an hour...
   
   Only the next day they are ready. As pointed out on the cloudstack-users 
list, I had to "clean up" the network after adding the egress rule to allow 
traffic to flow, but that's another story.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974407
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 for ((i=0;i<$RETRY_COUNT;i++))
 do
-  pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl 
mock flask netaddr pylint pep8 > /tmp/piplog
+  pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock 
flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog
 
 Review comment:
   @rafaelweingartner are we using new pkgs - `six astroid Markdown`, where/how?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974273
 
 

 ##
 File path: tools/travis/before_install.sh
 ##
 @@ -97,10 +99,11 @@ echo "
 echo -e "\nInstalling some python packages: "
 
 pip install --user --upgrade pip
+pip uninstall pylint
 
 Review comment:
   @rafaelweingartner Why uninstall here? I see `pylint` is installed again by 
pip at line 106.


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

rhtyd commented on a change in pull request #2576: Fix Python code checkstyle 
execute by "systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974168
 
 

 ##
 File path: systemvm/test/runtests.sh
 ##
 @@ -21,9 +21,9 @@
 export PYTHONPATH="../debian/opt/cloud/bin/"
 export PYTHONDONTWRITEBYTECODE=False
 
-echo "Running pep8 to check systemvm/python code for errors"
-pep8 --max-line-length=179 *py
-pep8 --max-line-length=179 
--exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py `find 
../debian -name \*.py`
+echo "Running pycodestyle to check systemvm/python code for errors"
+pycodestyle --max-line-length=179 *py
 
 Review comment:
   @rafaelweingartner Why not retain using pep8?


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381874376
 
 
   Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1936


This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"

[GitBox]

blueorangutan commented on issue #2576: Fix Python code checkstyle execute by 
"systemvm\test\runtests.sh"
URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381868051
 
 
   @rafaelweingartner 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 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