[GitHub] cloudstack pull request: CLOUDSTACK-8648: Do not configure the Ima...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/606#issuecomment-123585585 Yes. Copy paste error :) -Original Message- From: "Wido den Hollander" Sent: ‎21-‎07-‎2015 21:19 To: "apache/cloudstack" Cc:

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread resmo
Github user resmo commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/601#discussion_r35187102 --- Diff: scripts/vm/network/security_group.py --- @@ -860,8 +860,10 @@ def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

2015-07-22 Thread pritisarap12
GitHub user pritisarap12 opened a pull request: https://github.com/apache/cloudstack/pull/613 CLOUDSTACK-8659: Verify presentation of volume ID in events table Verify if events table records UUID of the volume instead of volume ID in description from which snapshot is created for 'S

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35188707 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java --- @@ -57,17 +57,22 @@ private LdapContextF

[GitHub] cloudstack pull request: CLOUDSTACK-8658: make initializer static ...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/612#issuecomment-123609068 LGTM :+1: merging... Thanks, @DaanHoogland !!! Tested with: KVM + Qemu on Centos7 Management Server on Centos7 Agent 4.6.0

[GitHub] cloudstack pull request: CLOUDSTACK-8658: make initializer static ...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/612 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

Re: [MASTER] ID RSA pub too open and asking passprrase

2015-07-22 Thread Wilder Rodrigues
After 3 days we managed to find the problem. It has been fixed and pushed towards master. KVM is back on business! Details here: https://github.com/apache/cloudstack/pull/612 Cheers, Wilder On 20 Jul 2015, at 15:51, Wilder Rodrigues mailto:wrodrig...@schubergphilis.com>> wrote: Hi Wido, I’m

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35191354 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java --- @@ -57,17 +57,22 @@ private LdapContextFacto

[Proposal] CoreOS support

2015-07-22 Thread Kishan Kavala
I would like to add CoreOS as a supported OS type to cloudstack. CoreOS vms can be created even now by selecting OS type as "Other". There are cloud providers supporting CoreOS offerings in production already. By making the changes mentioned at [1] I would make it easier to deploy CoreOS Vms and

[GitHub] cloudstack pull request: CLOUDSTACK-8655: [Browser Based Upload Vo...

2015-07-22 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/611#issuecomment-123636384 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

RE: [PROPOSAL] drop old upgrade code

2015-07-22 Thread Koushik Das
-1 to dropping pre-4.x upgrade code. If possible we should suppress the old upgrade files from coverity scan. Reasons: There may be users on pre-4.x versions. Removing a functionality should be associated with proper documentation and an advanced notification in some prior releases. This is simi

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread borisroman
GitHub user borisroman opened a pull request: https://github.com/apache/cloudstack/pull/614 CLOUDSTACK-8649: Fixed unnecessary double url decoding in registerSSHKeyPair. The method cmd.getPublicKey() already returns a decoded string. So when we try to decode it again the plus "+" s

RE: [PROPOSAL] drop old upgrade code

2015-07-22 Thread Boris Schrijver
+1 on dropping the pre-4.x upgrade code, if done in a documented manner. Instead of voting to drop it now shall we vote to drop it in a future release with documentation and put it on the roadmap? Like: At release 4.6: Initial notice to drop pre-4.x upgrade code at release 5.0. At release 4.6: Sup

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

2015-07-22 Thread nitt10prashant
Github user nitt10prashant commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/613#discussion_r35197771 --- Diff: test/integration/testpaths/testpath_uuid_event.py --- @@ -0,0 +1,198 @@ +# Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/614#issuecomment-123650029 Hi @borisroman , Should this go into 4.5 first? If so, why? Cheers, Wilder --- If your project is set up for it, you can reply to this

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread borisroman
Github user borisroman closed the pull request at: https://github.com/apache/cloudstack/pull/614 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread borisroman
GitHub user borisroman opened a pull request: https://github.com/apache/cloudstack/pull/615 CLOUDSTACK-8649: Fixed unnecessary double url decoding in registerSSHKeyPair. The method cmd.getPublicKey() already returns a decoded string. So when we try to decode it again the plus "+" s

[GitHub] cloudstack pull request: CLOUDSTACK-8658: make initializer static ...

2015-07-22 Thread borisroman
Github user borisroman commented on the pull request: https://github.com/apache/cloudstack/pull/612#issuecomment-123661436 The https://builds.apache.org/job/cloudstack-pull-requests fail after adding the Default Charset test. I think the localization of the Jenkins server needs to be

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/609#issuecomment-123668417 ``` root@Rajani's MS ~/source/cloudstack/plugins/user-authenticators/ldap (CLOUDSTACK-8596)$ mvn clean test [INFO] Scanning for projects... [INFO] [I

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/609#issuecomment-123668544 @DaanHoogland now you can run the tests from the command line or ide --- If your project is set up for it, you can reply to this email and have your reply appear o

Re: Review Request for PR 609

2015-07-22 Thread Rajani Karuturi
I ran them from intellij (right click on the test package, run tests) It also used to work from command line (mvn test from the ldap project) Its not working currently due to the commit ec32ea30f7b3e5351e661786955d9fa0929047bd which changed gmaven plugin version. I will revert that and update the

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread wilderrodrigues
GitHub user wilderrodrigues opened a pull request: https://github.com/apache/cloudstack/pull/616 CLOUDSTACK-8660 - StringUtilsTest.testGetPrefferedCharset() fails because the preferred charset is not always UTF-8 Test was failing due to "expected: but was:" Build is success

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/601#issuecomment-123697692 @bhaisaab I was going to ask @franklouwers how did he test his changes, since I would like to test them before giving a LGTM. Actually, I think you w

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123699898 @DaanHoogland double URL decoding was added during registerSSHKeyPair refactor. commit 968e71ad0e49088b5f2f022df29aec02b10a5ede Can you please check if this

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/616#issuecomment-123701878 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature ena

[GitHub] cloudstack pull request: CLOUDSTACK-8655: [Browser Based Upload Vo...

2015-07-22 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/611#issuecomment-123703777 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread franklouwers
Github user franklouwers commented on the pull request: https://github.com/apache/cloudstack/pull/601#issuecomment-123706604 All, I'll check the typo (I know how it happend) later this afternoon. Will also provide logs both before (bad behaviour: rule not installed) and after

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread bhaisaab
Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/601#issuecomment-123711497 @wilderrodrigues yeah, as I said I've not tested it. Just had a glance at the code, but good that @resmo pointed out the typo. --- If your project is set up for it

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/616#issuecomment-123721624 The other test in the string utils test class also failed. I will have a look at that one as well. Cheers, Wilder Sent from my i

[GitHub] cloudstack pull request: CLOUDSTACK-8655: [Browser Based Upload Vo...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/611 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

Re: [Proposal] CoreOS support

2015-07-22 Thread Wido den Hollander
On 07/22/2015 11:14 AM, Kishan Kavala wrote: > I would like to add CoreOS as a supported OS type to cloudstack. > CoreOS vms can be created even now by selecting OS type as "Other". There are > cloud providers supporting CoreOS offerings in production already. > By making the changes mentioned a

Re: [Proposal] CoreOS support

2015-07-22 Thread Koushik Das
+1 On 22-Jul-2015, at 2:44 PM, Kishan Kavala wrote: > I would like to add CoreOS as a supported OS type to cloudstack. > CoreOS vms can be created even now by selecting OS type as "Other". There are > cloud providers supporting CoreOS offerings in production already. > By making the changes men

Re: [Proposal] CoreOS support

2015-07-22 Thread Ahmad
+1 looking forward to this. > On Jul 22, 2015, at 1:14 PM, Kishan Kavala wrote: > > I would like to add CoreOS as a supported OS type to cloudstack. > CoreOS vms can be created even now by selecting OS type as "Other". There are > cloud providers supporting CoreOS offerings in production alre

RE: [Proposal] CoreOS support

2015-07-22 Thread Somesh Naidu
+1 Regards, Somesh -Original Message- From: Ahmad [mailto:aemne...@gmail.com] Sent: Wednesday, July 22, 2015 10:28 AM To: dev@cloudstack.apache.org Subject: Re: [Proposal] CoreOS support +1 looking forward to this. > On Jul 22, 2015, at 1:14 PM, Kishan Kavala wrote: > > I would lik

Re: [Proposal] CoreOS support

2015-07-22 Thread Sebastien Goasguen
+1 Can you clean that last of OS Type while you are it, last time I checked it was really long with some outdated distros… -sebastien > On Jul 22, 2015, at 4:35 PM, Somesh Naidu wrote: > > +1 > > Regards, > Somesh > > -Original Message- > From: Ahmad [mailto:aemne...@gmail.com] > S

Re: [Proposal] CoreOS support

2015-07-22 Thread Simon Weller
+1 as well. From: Sebastien Goasguen Sent: Wednesday, July 22, 2015 9:39 AM To: dev@cloudstack.apache.org Subject: Re: [Proposal] CoreOS support +1 Can you clean that last of OS Type while you are it, last time I checked it was really long with some o

Re: [Proposal] CoreOS support

2015-07-22 Thread Keerthiraja SJ
Will this works even for the RancherOS On Wed, Jul 22, 2015 at 8:24 PM, Simon Weller wrote: > > +1 as well. > > > From: Sebastien Goasguen > Sent: Wednesday, July 22, 2015 9:39 AM > To: dev@cloudstack.apache.org > Subject: Re: [Proposal] CoreOS support >

RE: [Proposal] CoreOS support

2015-07-22 Thread Suresh Sadhu
Right now No +1 for core os support Regards sadhu -Original Message- From: Keerthiraja SJ [mailto:sjkeer...@gmail.com] Sent: 22 July 2015 20:32 To: dev@cloudstack.apache.org Subject: Re: [Proposal] CoreOS support Will this works even for the RancherOS On Wed, Jul 22, 2015 at 8:24 PM,

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread franklouwers
Github user franklouwers commented on the pull request: https://github.com/apache/cloudstack/pull/601#issuecomment-123756479 See updated commit to fix the missing : . See also https://gist.github.com/franklouwers/d5061b4ef50e2b4253fe with logs of what works, what doesn't work,

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/616#issuecomment-123758576 Build is back to normal... keys are being created... and VMs are also fine. See details below: @karuturi @bhaisaab @DaanHoogland: got some time for a

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123766531 Hi guys, 1. RegisterSSHKeyPairCmd.getPublicKey() doesn't do a decoding of the key. That's just a simple getter method. 2. However, I don't think

[GitHub] cloudstack pull request: CLOUDSTACK-7539: coverity regression dead...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/610#issuecomment-123775323 LGTM :+1: Merging... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does no

[GitHub] cloudstack pull request: CLOUDSTACK-7539: coverity regression dead...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/610 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] cloudstack pull request: Dockerfile

2015-07-22 Thread runseb
Github user runseb commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/605#discussion_r35232143 --- Diff: tools/docker/Dockerfile.marvin --- @@ -0,0 +1,37 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor l

[GitHub] cloudstack pull request: Dockerfile

2015-07-22 Thread runseb
Github user runseb commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/605#discussion_r35232181 --- Diff: tools/docker/README.md --- @@ -0,0 +1,66 @@ + +# Docker Files + +Dockerfiles used to build CloudStack images available on Docker h

[GitHub] cloudstack pull request: Dockerfile

2015-07-22 Thread runseb
Github user runseb commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/605#discussion_r35232136 --- Diff: tools/docker/Dockerfile.centos6 --- @@ -0,0 +1,60 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor

[GitHub] cloudstack pull request: Dockerfile

2015-07-22 Thread runseb
Github user runseb commented on the pull request: https://github.com/apache/cloudstack/pull/605#issuecomment-123779835 LGTM if you address my couple comments. And really, I would squash this (33 commits), I don't see why we need to keep all those tiny commits in our master branch.

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread teulaert
Github user teulaert commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123818145 @wilderrodrigues reproducing is really easy, just try to register your public key using the registerSSHKeyPair API call. In the database you will find you key, but

[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...

2015-07-22 Thread manuiiit
GitHub user manuiiit opened a pull request: https://github.com/apache/cloudstack/pull/617 Added a null value exception in LibvirtBackupSnapshotCommandWrapper.java You can merge this pull request into a Git repository by running: $ git pull https://github.com/manuiiit/cloudstac

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123863689 Hi Lennert, The Command claases use annotations for the fields. The fields values are set via reflection. I bet my iPhone the decoding is being done

[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/617#issuecomment-123879270 You added code to a class that was recently refactored and was covered by unit tests. Please add a new unit test to cover the new flow. Cheers, W

Fwd: Re: XenServer is disconnected after CS hosts shutdown

2015-07-22 Thread tony_caotong
copy mail thread to @dev for seeking more help. Forwarded Message Subject:Re: XenServer is disconnected after CS hosts shutdown Date: Wed, 22 Jul 2015 21:03:13 +0800 From: tony_caot...@163.com Reply-To: us...@cloudstack.apache.org To: us...@cloudstack.apa

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/609#issuecomment-123954109 @DaanHoogland @abhinandanprateek @runseb @wilderrodrigues @bhaisaab Can you please review this PR? --- If your project is set up for it, you can reply to this emai

[GitHub] cloudstack pull request: Dereference NULL return value

2015-07-22 Thread kansal
GitHub user kansal opened a pull request: https://github.com/apache/cloudstack/pull/618 Dereference NULL return value No check on the null value of metadatafile. If null, the following operations failed. Added null check for the metadatafile. You can merge this pull request into a

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

2015-07-22 Thread sanju1010
Github user sanju1010 commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/613#discussion_r35288952 --- Diff: test/integration/testpaths/testpath_uuid_event.py --- @@ -0,0 +1,198 @@ +# Licensed to the Apache Software Foundation (ASF) under one +

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

2015-07-22 Thread sanju1010
Github user sanju1010 commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/613#discussion_r35288977 --- Diff: test/integration/testpaths/testpath_uuid_event.py --- @@ -0,0 +1,198 @@ +# Licensed to the Apache Software Foundation (ASF) under one +

Re: [GitHub] cloudstack pull request: CLOUDSTACK-8634: Made changes to test_sec...

2015-07-22 Thread Sanjeev N
Can somebody please review this PR? On Tue, Jul 14, 2015 at 5:58 PM, sanju1010 wrote: > GitHub user sanju1010 opened a pull request: > > https://github.com/apache/cloudstack/pull/586 > > CLOUDSTACK-8634: Made changes to test_security_group.py test suite to > support EIP > > > Made ch

[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/617#discussion_r35289154 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java --- @@ -25,6 +25,8 @@ impo

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/616#issuecomment-123959991 looks good :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] cloudstack pull request: Dereference NULL return value

2015-07-22 Thread kishankavala
Github user kishankavala commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/618#discussion_r35290636 --- Diff: plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java --- @@ -368,8 +368,14 @@ private boolean setup(

[PROPOSAL] Trust LDAP and Auto Import of users

2015-07-22 Thread Rajani Karuturi
We(Sarath and me) are working on adding support for auto import of LDAP users to a configured domain in CloudStack. A WIP FS is available at https://cwiki.apache.org/confluence/display/CLOUDSTACK/WIP%3A+LDAP%3A+Trust+AD+and+Auto+Import Please review and let us know if there are any comments/sugge

[GitHub] cloudstack pull request: CLOUDSTACK-8634: Made changes to test_sec...

2015-07-22 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/586#issuecomment-123971770 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] cloudstack pull request: CLOUDSTACK-8651: [Browser Based Upload Te...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/607#discussion_r35290816 --- Diff: test/integration/component/test_browse_templates.py --- @@ -1674,6 +1674,42 @@ def test_09_Browser_Upload_Volume_secondary_storage_resource_lim

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35291227 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java --- @@ -36,6 +36,8 @@ private static

[GitHub] cloudstack pull request: Dereference NULL return value

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/618#discussion_r35291257 --- Diff: plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java --- @@ -368,8 +368,14 @@ private boolean setup() {

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35291594 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java --- @@ -36,6 +36,8 @@ private static fin

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35291585 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35291761 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to th

[GitHub] cloudstack pull request: CLOUDSTACK-8659: Verify presentation of v...

2015-07-22 Thread pritisarap12
Github user pritisarap12 commented on the pull request: https://github.com/apache/cloudstack/pull/613#issuecomment-123986839 Updated the testcase as per review comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/617#issuecomment-123987028 Hi @manuiiit I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage, which is still poor compared to the other classes in the KVM

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/616#issuecomment-123987490 Thanks for the LGTM, @karuturi :) 2 LGTM, merging... --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-8660 - StringUtilsTest.testGet...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/616 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292477 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292541 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292608 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java --- @@ -57,17 +57,22 @@ private LdapContextFa

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123990021 Okay, we got a LGTM from @kishankavala and 1 from me. The parameters are being decoded in the ApiServlet.processRequestInContext() method. C

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/615 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123990422 encoding is done from javascript https://github.com/apache/cloudstack/blob/master/ui/scripts/accounts.js#L1816 I think we should either update the apidoc to send

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123991576 Hi @karuturi I merged just 1min before your comment. What change do you mean? The one @borisroman is trying to fix? Please, see my

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292815 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to th

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292860 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java --- @@ -0,0 +1,81 @@ +/* + * Licensed to th

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35292915 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java --- @@ -57,17 +57,22 @@ private LdapContextFacto

[GitHub] cloudstack pull request: CLOUDSTACK-8649: Fixed unnecessary double...

2015-07-22 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/615#issuecomment-123995367 ok.I thought thought this commit removed decoding and the api now expects a decoded value as input. I understand now. the issue is with double decoding we remov

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/601#issuecomment-123995530 Awesome @franklouwers ! Thanks for the details. LGTM :+1: Merging... --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

2015-07-22 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/601 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35293493 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java --- @@ -0,0 +1,55 @@ +/* + * Licensed t

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35293580 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java --- @@ -0,0 +1,55 @@ +/* + * Licensed t

[GitHub] cloudstack pull request: CLOUDSTACK-8580: user can view, expunge a...

2015-07-22 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/593#issuecomment-123996760 @wido @DaanHoogland @borisroman @runseb I will test it manually before proceeding with a merge. I will also add an integration test for that and sen

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35293652 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java --- @@ -0,0 +1,233 @@ +// Licensed to the

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35293673 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java --- @@ -0,0 +1,55 @@ +/* + * Licensed to t

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread koushik-das
Github user koushik-das commented on the pull request: https://github.com/apache/cloudstack/pull/609#issuecomment-123998336 Once the comments are addressed, LGTM. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your pr

[GitHub] cloudstack pull request: CLOUDSTACK-8596 ability to query nested g...

2015-07-22 Thread karuturi
Github user karuturi commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/609#discussion_r35294032 --- Diff: plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java --- @@ -0,0 +1,55 @@ +/* + * Licensed to t

[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...

2015-07-22 Thread manuiiit
Github user manuiiit commented on the pull request: https://github.com/apache/cloudstack/pull/617#issuecomment-123998699 Hi Wilder, The LibvirtBackupSnapshotCommandWrapper class has no unit tests.Can I add test cases that covers the exception. Regards, P.M