[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-19 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/715 --- 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-8687: Prepare template only on...

2015-08-19 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/715#issuecomment-132532173 :+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-8687: Prepare template only on...

2015-08-19 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/715#issuecomment-132530052 travis builds, awaiting analysis build but LGTM --- 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-8687: Prepare template only on...

2015-08-18 Thread devdeep
GitHub user devdeep opened a pull request: https://github.com/apache/cloudstack/pull/715 CLOUDSTACK-8687: Prepare template only on a given storage pool Update prepare template api to seed/prepare a template only on a given primary storage. Currently, the prepare template api will se

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-18 Thread devdeep
Github user devdeep closed the pull request at: https://github.com/apache/cloudstack/pull/635 --- 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-8687: Prepare template only on...

2015-08-18 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-132439804 @remibergsma I'll close this pull request and generate a new one where the unit tests are squashed in one commit. --- If your project is set up for it, you can repl

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-18 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-132237794 @devdeep Could you please make your commit message (first line) more descriptive? Right now I cannot tell the difference between them, unless I click on them and

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-18 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-132237051 Travis error is due to timeout. --- 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 d

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-18 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-132174594 @devdeep look great. one question but 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 yo

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-17 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-132072881 @DaanHoogland added some additional tests to validate the templates get scheduled for seeding. --- If your project is set up for it, you can reply to this email and

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-14 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-131242854 @devdeep do you have plans to add further testing? I think for the sake of testing you can change the encapsulation level. --- If your project is set up for it

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-14 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-131020409 Added another test to validate the prepareTemplate function. Looked into if it was possible to mock the threadpool executor service in templatemanagerimpl. However,

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-10 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-129412845 You can mock the call and verify how often it has been called with mockito. --- If your project is set up for it, you can reply to this email and have your repl

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-10 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-129411154 @DaanHoogland That is what I am looking into; how to get the number of threads that have been started by the executor service. --- If your project is set up for it,

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-10 Thread remibergsma
Github user remibergsma commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r36620562 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description = "

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-10 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-129408998 @devdeep How about testing if the right (number of) threads are being started? Does my comment make any sense? --- If your project is set up for it, you can re

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-10 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r36613344 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description =

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-09 Thread remibergsma
Github user remibergsma commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r36592822 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description = "

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-09 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-129228698 @DaanHoogland So, you suggest putting the new tests in a separate PR and add relevant unit tests that test the change in this PR? That sounds good to me. Let's c

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-07 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-128843113 @devdeep You have written a very fine test and as it passes and tests what its target should do you would get a definate lgtm on tat one from me. However it doe

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-06 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-128292255 @DaanHoogland When prepareTemplate is called, it either falls into prepareTemplateInOneStoragePool if one primary has to be seeded with the template; or prepareTempl

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-05 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-127946635 @devdeep the test you added seems fine and passes (the assert messages look like they can be better formulated to describe what went wrong) I do not understa

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-05 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r36286725 --- Diff: server/test/com/cloud/template/TemplateManagerImplTest.java --- @@ -18,20 +18,369 @@ package com.cloud.template; -impor

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-08-05 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-127918187 will do but a bit busy today, hang on --- 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-8687: Prepare template only on...

2015-08-04 Thread devdeep
Github user devdeep commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-127850003 @DaanHoogland Added unit tests for prepare template functionality too. Can you take a look. --- If your project is set up for it, you can reply to this email and ha

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-07-30 Thread kishankavala
Github user kishankavala commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r35862098 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description =

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-07-30 Thread devdeep
Github user devdeep commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r35849928 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description = "temp

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-07-30 Thread kishankavala
Github user kishankavala commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/635#discussion_r35849002 --- Diff: api/src/org/apache/cloudstack/api/command/admin/template/PrepareTemplateCmd.java --- @@ -60,6 +61,15 @@ description =

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-07-29 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/635#issuecomment-125917105 please add unit- or integration tests. for instance for prepareTemplate, prepareTemplateInOneStoragePool and prepareTemplateInAllStoragePools to show that expec

[GitHub] cloudstack pull request: CLOUDSTACK-8687: Prepare template only on...

2015-07-29 Thread devdeep
GitHub user devdeep opened a pull request: https://github.com/apache/cloudstack/pull/635 CLOUDSTACK-8687: Prepare template only on a given storage pool Update prepare template api to seed/prepare a template only on a given primary storage. Currently, the prepare template api will se