Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-222499981
This is a bit of a grey area. I will consider it, but I need jenkins and
travis coming back clean to be comfortable with merging and I will have to run
another CI
Github user sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-222489855
This will help in stabilizing CI runs.
---
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 sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-222489678
@swill , can you consider marvin test related changes for merging into ACS
master since they would not destabilize the master?
---
If your project is set up
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-222487710
Master is currently frozen. I am merging bug fixes if they are needed to
stabilize master, but I don't think this qualifies...
---
If your project is set up for
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-219539120
@sanju1010 we have merge conflicts now. Can you please rebase against the
current master. Code freeze is basically now, so if you want this in we need to
get busy...
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-218807584
@sanju1010 we have merge conflicts now. Can you please rebase against the
current master. Thanks...
---
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/1455#issuecomment-218366880
Bump... @sanju1010 can you please follow up on @DaanHoogland's comments.
This one is ready to go otherwise, so it would be great if you could follow up
with us on
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-216887066
@sanju1010 can you address @DaanHoogland's concerns?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-215512619
@sanju1010 though the code looks alright I don't like the integration test
should be snooping under the hood. I would rather see them behave as black-box
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-215481337
Yes, I think this one is ready... Can I get one more LGTM code review on
this one?
@pdube @kiwiflyer @DaanHoogland @rafaelweingartner
---
If your
Github user sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-215427658
@swill , it is all green now. Can you please merge this PR ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-215266469
I have tested this and I think we are ready. Can you please do a force
push to kick off jenkins again so we can get this PR all green? Thanks...
---
If your
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-215266162
### CI RESULTS
```
Tests Run: 4
Skipped: 0
Failed: 0
Errors: 0
```
**Associated Uploads**
Github user sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-206295391
@swill , I have made changes based on the CI results.
Following failure in the CI run is not related to the changes in this PR:
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-205111794
### CI RESULTS
**MULTIPLE FAILURES, NEEDS WORK!**
There are problems with this code. Here is a summary of the issues. I
have
Github user sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-204248348
@alexandrelimassantana , cleanup does not clean the db connection. So it
should work for other tests as well.
---
If your project is set up for it, you can
Github user eriweb commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-203855979
@remibergsma @wilderrodrigues @borisroman you guys have been testing a lot
previously, would this change work for you?
---
If your project is set up for it, you
Github user shwetaag commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-203818359
Did Code walk through . 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 your project does
Github user alexandrelimassantana commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-202874549
Does the _cleanup_resources(cls.api_client, cls._cleanup)_ cleans the db
connection as well? I see that it is only 1 test as of now, but if there
Github user sanju1010 commented on the pull request:
https://github.com/apache/cloudstack/pull/1455#issuecomment-202342204
Made changes as per Shwetaag's comments.
---
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 shwetaag commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1455#discussion_r57562054
--- Diff: test/integration/smoke/test_privategw_acl.py ---
@@ -287,6 +298,16 @@ def performVPCTests(self, vpc_off,
restart_with_cleanup = False):
GitHub user sanju1010 opened a pull request:
https://github.com/apache/cloudstack/pull/1455
[CLOUDSTACK-9328]: Fix vlan issues from test suite test_privategw_acl.py in
BVT
Please refer to CLOUDSTACK-9328 for the details.
Test Results:
==
22 matches
Mail list logo
