Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1532
---
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 user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-219158465
@swill you should always count the opinion of the RM as that of a real
memeber of the community ;)
---
If your project is set up for it, you can reply to this
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-219093207
I am counting my own code review on this one given the scope of the change
and the fact that a review is the only thing holding this up. I will merge
this one...
-
Github user marcaurele commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-219010730
checks are finally ok...
---
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
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218879748
Sorry to do this to you again @marcaurele. Can you rebase and force push
again. I have disabled the test in `master` that was blocking this in Jenkins
because we ha
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218806166
@marcaurele I have merged fixes to Jenkins and Travis, can you force push
again to kick off those runs again. Sorry for the inconvenience, hopefully
this is the last
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218742950
Once I get #1539 merged, I will ask you to re-push since we are working to
fix the issue that is causing Travis to fail in that PR right now. Thx...
---
If your pro
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218732878
@marcaurele I don't think it is related to your code either. can you
open/close your pr to see if it works. If not, maybe we need to empty the
jenkins-workspac
Github user marcaurele commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218731447
I don't get what's the relation between my change and the jenkins build
failling. Should I trigger another one?
---
If your project is set up for it, you can re
GitHub user marcaurele reopened a pull request:
https://github.com/apache/cloudstack/pull/1532
DAO: Hit the cache for entity flagged as removed too
I came along this part of the code and I don't see any reason why the cache
should not be used when fetching with the "removed" ones. I
Github user marcaurele closed the pull request at:
https://github.com/apache/cloudstack/pull/1532
---
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 featur
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218472093
@marcaurele instead of force push you can close and reopen after a few
seconds. This usually works as well and seves you switching windows
browser->terminal->b
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218456049
Thank you sir. Unfortunately travis failed this time. Would you mind
trying it again. Thanks...
---
If your project is set up for it, you can reply to this email
Github user marcaurele commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218370950
@swill did a force push by changing the last commit description
---
If your project is set up for it, you can reply to this email and have your
reply appear on G
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218367932
I reran the tests that failed and they succeeded the second time, so CI is
in a good place on this one. I need one more code review on this one.
@marcaurele can you
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218367555
### CI RESULTS
```
Tests Run: 94
Skipped: 0
Failed: 2
Errors: 0
Duration: 7h 45m 13s
```
**Summary of the p
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218169825
@DaanHoogland I am also getting more false negatives than I had in the
past. I am also pushing my environments a bit harder than I was before, so I
think that is par
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218162701
@swill I am getting to many false positives in my environment to grant it
value, can you schedule this for integration testing?
---
If your project is set up
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-218003768
Thanks @DaanHoogland. ð
---
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
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217866994
LGTM based on code, CI pending
---
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 DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62482644
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -942,12 +942,18 @@ public T findOneBy(final SearchCriteria sc) {
@D
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217829031
@marcaurele @sateesh-chodapuneedi You are both right probably more then I
will ever be so please take with a grain of salt:
```
T result = null;
Github user marcaurele commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217820629
I pushed a change to write the method following the good practive you're
mentioning. I also updated the sibling method findById to follow the same
practice.
--
Github user marcaurele commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217820309
@sateesh-chodapuneedi not quite, you're missing a case. If the cache is
null (not configured) you will always return `null`, which is not the goal.
---
If your
Github user sateesh-chodapuneedi commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217814911
Probably intention is to,
```java
T result = null
if (_cache != null) {
final Element element = _cache.get(id);
Github user sateesh-chodapuneedi commented on the pull request:
https://github.com/apache/cloudstack/pull/1532#issuecomment-217814496
@DaanHoogland I agree that's the best practice though there is a glitch
here. Following code returns null if _cache is not null but element is
Github user drsridevi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62469111
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62456988
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user marcaurele commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62453258
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62349471
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62335611
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62303518
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1532#discussion_r62303317
--- Diff: framework/db/src/com/cloud/utils/db/GenericDaoBase.java ---
@@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid)
{
GitHub user marcaurele opened a pull request:
https://github.com/apache/cloudstack/pull/1532
DAO: Hit the cache for entity flagged as removed too
I came along this part of the code and I don't see any reason why the cache
should not be used when fetching with the "removed" ones. It
34 matches
Mail list logo
