Github user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1491
---
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 featu
GitHub user ProjectMoon reopened a pull request:
https://github.com/apache/cloudstack/pull/1491
Fixes regarding VOLUME_DELETE events resulting from account deletion.
New version of #1373, but updated for the 4.7 branch with another fix that
allows it to properly find expunged root v
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-221916927
@ProjectMoon and again. :( sorry...
---
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 user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1491
---
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 featu
GitHub user ProjectMoon reopened a pull request:
https://github.com/apache/cloudstack/pull/1491
Fixes regarding VOLUME_DELETE events resulting from account deletion.
New version of #1373, but updated for the 4.7 branch with another fix that
allows it to properly find expunged root v
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-221873700
@ProjectMoon try to close and reopen this one to try to get it green.
thx...
---
If your project is set up for it, you can reply to this email and have your
reply a
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-221846283
Rebased against latest 4.7.
---
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 user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-221763412
This one needs review as well...
---
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 user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-218858373
yep, sounds good...
---
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 user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-218848337
so @swill, I may find time to review later. In the meanwhile let's use
tag:needsreview
makes sense?
---
If your project is set up for it, you
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-218838932
Can I get some code review on this PR? Thx...
---
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 user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-218838753
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 2
Errors: 0
Duration: 4h 19m 11s
```
**Summary of the p
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1491#discussion_r60003773
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -677,6 +679,17 @@ public boolean deleteAccount(AccountVO account, long
c
Github user cristofolini commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-211140525
@ProjectMoon May I suggest extracting lines 778-784 in `AccountManagerImpl`
to a separate method? That would allow you to write your comment in a proper
javado
Github user nnesic commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-209922376
@koushik-das We are mostly working with 4.7 version, where other volume
usage events get emitted correctly. We haven't investigated the behavior on
master yet.
--
Github user nnesic commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1491#discussion_r59707938
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -761,6 +774,17 @@ protected boolean cleanupAccount(AccountVO account,
long callerUserId
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1491#issuecomment-209830019
@ProjectMoon On latest master during VM deployment no VOLUME.CREATE is
getting generated for ROOT volume. Also during destroy there is no
VOLUME.DELETE event. O
Github user pdube commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1491#discussion_r59644462
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -761,6 +774,17 @@ protected boolean cleanupAccount(AccountVO account,
long callerUserId,
Github user pdube commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1491#discussion_r59644306
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -761,6 +774,17 @@ protected boolean cleanupAccount(AccountVO account,
long callerUserId,
GitHub user ProjectMoon opened a pull request:
https://github.com/apache/cloudstack/pull/1491
Fixes regarding VOLUME_DELETE events resulting from account deletion.
New version of #1373, but updated for the 4.7 branch with another fix that
allows it to properly find expunged root vol
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-209361378
We have found that this fix actually does not work on 4.7+. We are going to
submit a new pull request based on 4.7 with a working fix.
---
If your project is s
Github user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1373
---
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 featu
Github user bvbharatk commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-198312562
### ACS CI BVT Run
**Sumarry:**
Build Number 110
Hypervisor xenserver
NetworkType Advanced
Passed=104
Failed=15
Skipped=4
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-182301032
Even for forward-merged bug fixes?
---
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 proje
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-182267349
@ProjectMoon Currently waiting for a new Release Manager for version 4.9 to
pick this up
---
If your project is set up for it, you can reply to this email and have yo
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-179181011
Who gets to be the authority on merging this?
---
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 user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-177461984
[1373.network.results.txt](https://github.com/apache/cloudstack/files/111219/1373.network.results.txt)
[1373.vpc.results.txt](https://github.com/apache/clo
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-176700390
Congratulations, the code is much better now.
I can give LGTM now without doubt
---
If your project is set up for it, you can reply to this email and
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-176670774
I added the suggested changes. Also, now that #1382 has been merged to 4.6,
the checks should succeed.
---
If your project is set up for it, you can reply to t
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-176270996
@ProjectMoon that is great,
Your code is ok,
I would just suggest you extracting the code from "AccountManagerImpl"
lines 769-778 to a method. Th
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1373#discussion_r51146522
--- Diff: server/test/com/cloud/user/AccountManagerImplTest.java ---
@@ -192,6 +205,11 @@
@Mock
VMSnapshotDao _vmSnapshotDao;
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1373#discussion_r51146580
--- Diff: server/test/com/cloud/user/AccountManagerImplTest.java ---
@@ -205,6 +223,11 @@
@Mock
private UserAuthenticator user
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 u
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 jus
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1373#issuecomment-176205293
Hi @ProjectMoon ,
Out of curiosity, why did you create the
"server/test/com/cloud/user/MockUsageEventDao.java" class?
---
If your project is set up f
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 you
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1372#issuecomment-175716111
@ProjectMoon :) Github need to fix this, really annoying
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
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 regard
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, or if the featu
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 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 re
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
r
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1372#issuecomment-175710489
@ProjectMoon do you want to merge this only on master or since 4.6/4.7?
Check the target branch of the PR.
---
If your project is set up for it, you can reply to
43 matches
Mail list logo
