Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1529
---
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 swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-217856530
All tests are done and I have the code reviews I need. Adding to merge
queue. Thx...
---
If your project is set up for it, you can reply to this email and have you
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-217772487
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 2
Errors: 1
Duration: 10h 39m 56s
```
**Summary of the
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-217323925
@dmabry thank you. This type of feedback is very useful for me (as the RM)
---
If your project is set up for it, you can reply to this email and have your
reply app
Github user dmabry commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-217322431
I know it doesn't really need my LGTM, but this commit definitely improves
the accuracy, and possibly the performance, of marvin and I'd personally like
to see this
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216968844
Thanks @jburwell and @shwetaag for the reviews!
@swill We are good to go from a review standpoint here. I will update the
commit SHA and re-push this
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216883803
@mike-tutkowski I agree with you about the use of assertions. A very nice
improvement in the quality of the test.
LGTM based on code review
---
If your
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216743316
@jburwell I actually like this better because it tells me specifically what
the particular failure is.
assert validationresult is not None, "
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216742996
@jburwell How's about this?
assert validationresult is not None and isinstance(validationresult, list)
and len(validationresult) == 3,\
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216735024
@mike-tutkowski I apologize for being behind on my review queue. I will
move #1403 up on my list and get to it as quickly as I can.
---
If your project is set up
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61979134
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61974541
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyper
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61974533
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyper
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61974484
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyper
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61974295
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyper
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61974235
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyper
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216696449
Thanks @jburwell While you're in a code-review mood, maybe you take a look
at #1403 again as that one is pretty high value to a bunch of people. :)
---
If y
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216684125
@mike-tutkowski the ``wait_until`` version looks very nice ;) A definite
improvement in the reliability of the test case.
I added a couple of minor commen
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61967545
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61967452
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61967329
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61967231
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1529#discussion_r61967100
--- Diff: tools/marvin/marvin/lib/base.py ---
@@ -2459,13 +2460,31 @@ def create(cls, apiclient, cluster, services,
zoneid=None, podid=None, hyperviso
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216678631
@jburwell Thanks for pointing out that utility method. I have updated the
code.
---
If your project is set up for it, you can reply to this email and have y
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216632803
@mike-tutkowski you may want to consider refactoring to use the
[``wait_until``
function](https://github.com/apache/cloudstack/blob/12d8e53dd54d83242b3dabf2151249a
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/1529#issuecomment-216444177
I tested this by walking through with the debugger when the delete method
on Host was invoked from a test script of mine.
---
If your project is set up for
GitHub user mike-tutkowski opened a pull request:
https://github.com/apache/cloudstack/pull/1529
Marvin: Replace a timer.sleep(30) with pulling logic
https://issues.apache.org/jira/browse/CLOUDSTACK-9374
From the ticket:
In the base.py file, there is a Host class wi
27 matches
Mail list logo
