[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user milamberspace commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r50086554 --- Diff: ui/scripts/docs.js --- @@ -1008,7 +1008,7 @@ cloudStack.docs = { }, // Add volume helpVolumeName: { -

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172807056 LGTM, the one error in the files below is a known error in the test environment and unrelated. This has been reviewed extensively.

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1319 --- 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-9132: API createVolume takes e...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172522583 At leaseweb we noticed the same behavior for networks. I propose we use the uuid as the name there as well. Will create a PR. --- If your project is set up

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r49997495 --- Diff: ui/scripts/docs.js --- @@ -1008,7 +1008,7 @@ cloudStack.docs = { }, // Add volume helpVolumeName: { -

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user milamberspace commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r50079832 --- Diff: ui/scripts/docs.js --- @@ -1008,7 +1008,7 @@ cloudStack.docs = { }, // Add volume helpVolumeName: { -

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r50081550 --- Diff: ui/scripts/docs.js --- @@ -1008,7 +1008,7 @@ cloudStack.docs = { }, // Add volume helpVolumeName: { -

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172376749 @nitin-maharana We can merge this once the integration tests have run. Will put it on my list. --- If your project is set up for it, you can reply to this

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-172376856 Sure @remi --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-171278817 @nitin-maharana I see two LGTM in the other PR based on code review. One without explanation and none based on testing. I can queue this but you don't want to

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r49551329 --- Diff: server/src/com/cloud/storage/VolumeApiServiceImpl.java --- @@ -476,6 +476,25 @@ public VolumeVO doInTransaction(TransactionStatus

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-171184133 @remibergsma @DaanHoogland : It has got 4 LGTMs. Is it going to be merged? Should it be reviewed by some more people? Thanks. --- If your project is set up

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user mike-tutkowski commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r49493043 --- Diff: server/src/com/cloud/storage/VolumeApiServiceImpl.java --- @@ -476,6 +476,25 @@ public VolumeVO doInTransaction(TransactionStatus

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user mike-tutkowski commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r49498772 --- Diff: server/src/com/cloud/storage/VolumeApiServiceImpl.java --- @@ -476,6 +476,25 @@ public VolumeVO doInTransaction(TransactionStatus

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1319#discussion_r49498039 --- Diff: server/src/com/cloud/storage/VolumeApiServiceImpl.java --- @@ -476,6 +476,25 @@ public VolumeVO doInTransaction(TransactionStatus

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-170825451 @mike-tutkowski : Yes, we decided to make random name as a preferred solution. You can refer the detail conversation on this PR #1273 . Thanks for

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-170804399 Just curious. If this is the expected behavior (mentioned above): It shouldn't create a volume with an empty name. Error should be returned.

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-170801193 @remibergsma : Any updates on this. Thanks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-169927533 Closing this PR as made a new PR #1319 (Against 4.7 which will be merged in master later). --- If your project is set up for it, you can reply to this

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana closed the pull request at: https://github.com/apache/cloudstack/pull/1273 --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-169927365 Reference PR #1273 (Against master) with 3 LGTMs. This PR (against 4.7) contains the same code change. --- If your project is set up for it, you can reply

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1319#issuecomment-169935900 Hi @remibergsma, I created all my pending PRs against 4.7. Can you please review once. Thanks. --- If your project is set up for it, you can reply to this

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

GitHub user nitin-maharana opened a pull request: https://github.com/apache/cloudstack/pull/1319 CLOUDSTACK-9132: API createVolume takes empty string for name parameter Steps to Reproduce: Create a volume using createVolume API where parameter name is

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-168489232 @remibergsma : My new PRs are failing in 4.7. With the same change it was successful in master. I think there is some issue with the branch. --- If your

Re: [GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Remi, If a fix can be made agains 4.6, let's do that. Merging forward is no biggy and we'll be helping more people this way. On Sun, Jan 3, 2016 at 10:39 AM, remibergsma wrote: > Github user remibergsma commented on the pull request: > >

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-168489874 @nitin-maharana See thread on dev-list: the errors are due to quota plugin failing since Jan 1st 2016. Once this is resolved, you can rebase.

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-168481572 @nitin-maharana Any bug fix should be against 4.7, any new feature against master. Bug fixes will be forward merged to master after it is merged to 4.7. ---

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-168527922 @remibergsma There are internal political issues that people may face with certain upgrade, be they with or without reason. --- If your project is set up for

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-168284288 @remibergsma : Shall I make this one against 4.7. I mean, should I make all pending PRs against 4.7? Is it not going to be merged? --- If your project

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167953466 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-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167931759 cc @remibergsma @bhaisaab --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167760979 @nitin-maharana now almost everything is ok. There is only one problem. the "org.apache.commons.lang.StringUtils.isBlank" also returns true if the

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167773312 Thanks a lot @rafaelweingartner , I learned many things. --- 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-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167765606 @rafaelweingartner : Thats cool. I went through the detail of isBlank, but could not catch. Thank you. I have updated. --- If your project is set up

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167768433 @nitin-maharana now the code is perfect. I can give a LGTM now. Sorry if I bothered you with a lot of changes. --- If your project is set

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167540899 @nitin-maharana I did not notice that the other PR was to 4.6, I thought it was to master. I would just ask you to use a Javadoc block over

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167721727 @rafaelweingartner : I have updated the branch. Can you please review the change. Thanks. --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-167484496 @rafaelweingartner : Thanks for reviewing the change. The reason to close the old PR is because it was merging with branch 4.6. I started this PR

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-166987980 @nitin-maharana, thanks for the update, I will just call your attention to a few points: First of all, why did you open a new PR? I know you

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-166830953 Sure @koushik-das. Thanks. --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-166556707 Reference(4.6) : https://github.com/apache/cloudstack/pull/1206 --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1206#issuecomment-166556945 Made a new PR https://github.com/apache/cloudstack/pull/1273 with master. --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

GitHub user nitin-maharana opened a pull request: https://github.com/apache/cloudstack/pull/1273 CLOUDSTACK-9132: API createVolume takes empty string for name parameter Steps to Reproduce: Create a volume using createVolume API where parameter name is empty.

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana closed the pull request at: https://github.com/apache/cloudstack/pull/1206 --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user koushik-das commented on the pull request: https://github.com/apache/cloudstack/pull/1273#issuecomment-166823449 @nitin-maharana HypervisorUtilsTest test have failed, please do a force push again. Code changes LGTM --- If your project is set up for it, you can reply

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1206#issuecomment-165757054 cc @koushik-das @kishankavala --- 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

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/1206#issuecomment-165076449 @rafaelweingartner : Thanks for the suggestion. I will follow the same to write test cases for this. @DaanHoogland : Should we make the Name field as

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1206#issuecomment-165081512 @nitin-maharana that would be my preference. In the UI a pop-up could explain the consequence of not entering a name. --- If your project is set up for it,

[GitHub] cloudstack pull request: CLOUDSTACK-9132: API createVolume takes e...

GitHub user nitin-maharana opened a pull request: https://github.com/apache/cloudstack/pull/1206 CLOUDSTACK-9132: API createVolume takes empty string for name parameter Steps to Reproduce: Create a volume using createVolume API where parameter name is empty.