[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-29 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-176671839 Should the three commits be squashed? Or left in three? I suggest perhaps splitting the unit test commit into two, and putting each unit test with its

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1373#issuecomment-176263770 @rafaelweingartner I have removed the MockUsageEventDao and replaced it with a normal Mockito mock of the UsageEventDao interface. --- If your project is set

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1378#issuecomment-176159834 I added the license header. Out of curiosity, I ran the `org.apache.rat:apache-rat-plugin:0.10:check` locally on my PR branch and it reported 59

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1378 Emit event UUIDs on template deletion When a template is deleted, it emits a usage event over the event bus. However, it does not emit the UUID or class name of the template over the event

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1378#issuecomment-176144240 Says a license is missing, but this only touches one existing file. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1373#issuecomment-176208728 I'm not the original author of this work, but I think it was just because a class for testing was needed. Looking at it now I'm not really sure why it can't

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1378#issuecomment-176194553 I added the license check to ActionEventUtilsTest and of course now the checks pass. But may it is a better idea to put the license in as a separate PR

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1378#issuecomment-176204562 After some discussion on IRC, going to remove the addition of the license header here and fix it in a separate PR. --- If your project is set up for it, you

[GitHub] cloudstack pull request: Emit event UUIDs on template deletion

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1378#issuecomment-176197509 It looks like it was some old Debian package build causing a bunch of extraneous files to be placed into the rat report. Cleaned up those and the report is now

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1373#issuecomment-176200921 Added the license header to the MockUsageEventDao class to fix rat report error. --- If your project is set up for it, you can reply to this email and have

[GitHub] cloudstack pull request: Add missing license header to ActionEvent...

2016-01-28 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1382 Add missing license header to ActionEventUtilsTest. The test class was merged without the license header. This commit fixes that problem. Also note that the license header exists

[GitHub] cloudstack pull request: Add missing license header to ActionEvent...

2016-01-28 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1381 Add missing license header to ActionEventUtilsTest. The test class was merged without the license header. This commit fixes that problem. Also note that the license header exists

[GitHub] cloudstack pull request: Add missing license header to ActionEvent...

2016-01-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1381#issuecomment-176207310 My god I swear GitHub's PR interface can die in a fire. Wrong base branch again. --- If your project is set up for it, you can reply to this email and have

[GitHub] cloudstack pull request: Add missing license header to ActionEvent...

2016-01-28 Thread ProjectMoon
Github user ProjectMoon closed the pull request at: https://github.com/apache/cloudstack/pull/1381 --- 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

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-27 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1372 Fixes regarding VOLUME_DELETE events resulting from account deletion. New version of #924, but on the right branch with the commits squashed. **Original pull request:** Fixes

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1372#issuecomment-175712011 Yes.. wrong branch again. GitHub's cross-fork interface tends to be a bit confusing for me due to how it loads when you change repos. Attempt #3 coming up

[GitHub] cloudstack pull request: Usage event fixes for deleted accounts

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/924#issuecomment-175704775 New request is located at https://github.com/apache/cloudstack/pull/1372. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1372#issuecomment-175706330 Which file is missing the license? Still don't see that in the Jenkins build. --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-27 Thread ProjectMoon
Github user ProjectMoon closed the pull request at: https://github.com/apache/cloudstack/pull/1372 --- 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

[GitHub] cloudstack pull request: Fixes regarding VOLUME_DELETE events resu...

2016-01-27 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1373 Fixes regarding VOLUME_DELETE events resulting from account deletion. New version of #924, but on the right branch with the commits squashed. Original pull request: Fixes

[GitHub] cloudstack pull request: Usage event fixes for deleted accounts

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/924#issuecomment-175705151 I don't have the close button, so this one needs to be closed again. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-175727599 License has been added. --- 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

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-175634193 Seems the Jenkins build is complaining about license missing. Don't see what file it is, but I assume it's the new unit test file? --- If your project is set

[GitHub] cloudstack pull request: Usage event fixes for deleted accounts

2016-01-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/924#issuecomment-175675521 Squashing and rebasing is definitely needed. But isn't this a bug fix though? And thus should go on to 4.6 and then be forward merged? --- If your project

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-26 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-175039929 I have created the unit tests. They are in one commit. The tests test the protected methods delegated to by getDefaultNetworkForCreation. It tests all 4

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-25 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-174517194 I've added the changes requested, minus the tests (still working on those). I am pushing the changes now for review. @pedro-martins The CollectionUtils library

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-25 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1360#discussion_r50702050 --- Diff: services/secondary-storage/controller/src/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java --- @@ -516,6 +517,79

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-25 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1360#issuecomment-174561850 Also, where is the best place to put tests for the secondary storage manager? Is there already a test for it? --- If your project is set up for it, you can

[GitHub] cloudstack pull request: Followup fix for #1162: Add support for n...

2016-01-22 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1330#issuecomment-173869256 Perhaps I can make a small example test that starts cloudstack-setup-management under systemd with both `--no-start` and without it, showing the difference

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-22 Thread ProjectMoon
Github user ProjectMoon closed the pull request at: https://github.com/apache/cloudstack/pull/1359 --- 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

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-22 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1359#issuecomment-173872098 OK. So just to be sure: it's oldest supported maintenance release for bug fixes and then master for any new features/refactoring? --- If your project is set

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-22 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1360 Refactor system VM default network creation Two small commits which moves the retrieval of the default network for the console proxy and the SSVM into a separate protected method. It's

[GitHub] cloudstack pull request: Refactor system VM default network creati...

2016-01-22 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1359 Refactor system VM default network creation Two small commits which moves the retrieval of the default network for the console proxy and the SSVM into a separate protected method. It's

[GitHub] cloudstack pull request: Followup fix for #1162: Add support for n...

2016-01-12 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1330 Followup fix for #1162: Add support for not (re)starting server after cloud-setup-management. This is a follow-up fix for pull request #1162, which added a `--no-start` option

[GitHub] cloudstack pull request: Followup fix for #1162: Add support for n...

2016-01-12 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1330#discussion_r49445279 --- Diff: python/lib/cloudutils/serviceConfigServer.py --- @@ -138,9 +138,8 @@ def checkHostName(): except: pass

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-04 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1162#discussion_r46671185 --- Diff: python/lib/cloudutils/serviceConfigServer.py --- @@ -135,9 +135,14 @@ def checkHostName

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-04 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1162#discussion_r46671835 --- Diff: python/lib/cloudutils/serviceConfigServer.py --- @@ -135,9 +135,14 @@ def checkHostName

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-04 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1162#discussion_r46672120 --- Diff: python/lib/cloudutils/serviceConfigServer.py --- @@ -135,9 +135,14 @@ def checkHostName

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-04 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1162#issuecomment-161926917 Removed the unintentional change to the initLoging (sic) method. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-03 Thread ProjectMoon
Github user ProjectMoon commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1162#discussion_r46618985 --- Diff: client/bindir/cloud-setup-management.in --- @@ -24,20 +24,23 @@ from cloudutils.globalEnv import globalEnv from

[GitHub] cloudstack pull request: Add support for not (re)starting server a...

2015-12-03 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1162 Add support for not (re)starting server after cloud-setup-management. This adds an option to the cloud-setup-management script to not start the management server after a successful

[GitHub] cloudstack pull request: Usage event fixes for deleted accounts

2015-11-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/924#issuecomment-160135926 What's the status of this pull request? Do we need to redo this PR on 4.6 instead? Also what are the thoughts about squashing some of these commits down

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-26 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159883807 Well, the target branch for this is master so it will be put in 4.7-SNAPSHOT, but it's also intended to be cherry-picked back into 4.6 (which I obviously do

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-26 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159887285 Since the branches are different, I will create a new branch off of 4.6, cherry-pick my commit to it, and then submit that as a new PR, which can

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-26 Thread ProjectMoon
Github user ProjectMoon closed the pull request at: https://github.com/apache/cloudstack/pull/ --- 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

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-26 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/1127 Fix event UUIDS missing on event bus Same as pull request #, but this time on the 4.6 branch for forward merging in accordance with [the wiki](https://cwiki.apache.org/confluence

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-26 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159889056 Closed in favor of #1127. --- 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

[GitHub] cloudstack pull request: PR1102 regression fix; half the annotatio...

2015-11-24 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/1110#issuecomment-159429530 This build problem happens to me on Java 1.7. Isn't this just a syntax error? The annotation markers are missing. --- If your project is set up for it, you

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-24 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159341450 Example with the fix: ``` { eventDateTime=2015-11-24 16:57:58 +, status=Completed, description=Successfully completed

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-24 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159219262 Currently unable to build the branch due to https://github.com/apache/cloudstack/pull/1110. What's the status of that request being merged in? --- If your

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-24 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159277838 The commit has been updated with a unit test. I also rebased this branch to latest master since my apparent accidental review of #1110 got the syntax error fix

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-24 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159231415 For now I will cherry-pick that commit and use it locally to build my tests. It will not become part of this pull request of course. --- If your project

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-23 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159024273 Integration tests use Marvin, right? --- 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: Usage event fixes for deleted accounts

2015-11-23 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/924#issuecomment-158961187 The PR has been fixed by the application of two more commits. Maybe squashing is in order before a merge. The abuse of reflection required to make

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-23 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/ Fix event UUIDS missing on event bus The fixing of CLOUDSTACK-8816 introduced a regression that removed the first class entities in the event bus description property. This is because

[GitHub] cloudstack pull request: Fix event UUIDS missing on event bus

2015-11-23 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/#issuecomment-159001385 To test, remote debug an in-memory event bus with a command like disable or enable static NAT. Check event description property before and after

[GitHub] cloudstack pull request: CLOUDSTACK-9003 Make VirtualMachineName i...

2015-10-30 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/988#issuecomment-152513166 While I haven't pushed new work to this PR yet, I do have a prototype of what I intend to do, and am looking for thoughts on it here. My plan

[GitHub] cloudstack pull request: Make VirtualMachineName into an injectabl...

2015-10-28 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/988#issuecomment-151810393 I can create a ticket, yes. I plan to push more work to this pull request today/tomorrow that moves the VirtualMachineName service and probably the UUID

[GitHub] cloudstack pull request: Make VirtualMachineName into an injectabl...

2015-10-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/988#issuecomment-151557578 Looks like build keeps crashing. Should I maybe wait until later to run it? --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: Make VirtualMachineName into an injectabl...

2015-10-27 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/988#issuecomment-151553160 I think I also plan on refactoring this pull request further to make the VirtualMachineNameService extensible similar to how some CloudStack adapters work, so

[GitHub] cloudstack pull request: Make VirtualMachineName into an injectabl...

2015-10-27 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/988 Make VirtualMachineName into an injectable service class This pull request refactors the VirtualMachineName class into an injectable dependency. It is one of the few remaining classes

[GitHub] cloudstack pull request: Allow custom command role ACL files on cl...

2015-06-18 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/354#issuecomment-113106273 Pull request branch has been updated. --- 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: Allow custom command role ACL files on cl...

2015-06-12 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/354#issuecomment-111475122 This would be a step-by-step way of how I tested the change. We will use mycommands.properties as the example. 1. Add the new properties file name

[GitHub] cloudstack pull request: Allow EC2 to be run from Maven properly

2015-06-12 Thread ProjectMoon
Github user ProjectMoon closed the pull request at: https://github.com/apache/cloudstack/pull/389 --- 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

[GitHub] cloudstack pull request: Allow EC2 to be run from Maven properly

2015-06-11 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/389 Allow EC2 to be run from Maven properly This pull request adds two runtime dependencies to the POM for awsapi so the [developer guide instructions](http://docs.cloudstack.apache.org/en

[GitHub] cloudstack pull request: Allow custom command role ACL files on cl...

2015-06-08 Thread ProjectMoon
Github user ProjectMoon commented on the pull request: https://github.com/apache/cloudstack/pull/354#issuecomment-109915316 Is there anything I can do to assist with the testing? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] cloudstack pull request: Allow PropertiesUtil to read from jar fil...

2015-06-04 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/358 Allow PropertiesUtil to read from jar files. ## Commit Message PropertiesUtil has code for reading from jar files, but the findConfigFile method will prevent it from ever returning

[GitHub] cloudstack pull request: Allow custom command role ACL files on cl...

2015-06-03 Thread ProjectMoon
GitHub user ProjectMoon opened a pull request: https://github.com/apache/cloudstack/pull/354 Allow custom command role ACL files on classpath in Static Role API Checker ## Commit Message This commit has a small refactoring of cloud-plugin-acl-static-role-based to allow

<    1   2   3