You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by romain-dartigues <gi...@git.apache.org> on 2016/05/31 13:24:54 UTC
[GitHub] cloudstack pull request: systemvm - Python code cleanup
GitHub user romain-dartigues opened a pull request:
https://github.com/apache/cloudstack/pull/1575
systemvm - Python code cleanup
While debugging an issue on the System VM (to be reported later once we will have validated it behaviour); I took the opportunity to make some minor code clean-up in the Python scripts in the System-VM.
Are you open to discuss about the future (improvements, conformance, automated testing) of thoses scripts? Have you something already planned on your side?
Regards.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/romain-dartigues/cloudstack systemvm-pylint
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1575.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1575
----
commit fc1b1c48ed1c345af0ed4d9c680dc079cb40749c
Author: Romain Dartigues <rd...@orange.com>
Date: 2016-05-31T10:13:45Z
systemvm: make pylint happy
Mostly removing unused modules, but also defined some missing variables.
commit 5d4c39184c103902108c3ac2e4f96f7ec5dc99c4
Author: Romain Dartigues <rd...@orange.com>
Date: 2016-05-31T10:54:33Z
systemvm: set_redundant.py non-working script
Deleting set_redundant.py as it can not work (call config.set_cl() and
config.get_cmdline() which are nowhere to be found).
commit e863e968fd8233d1df929b3d333c87fccfda0c18
Author: Romain Dartigues <rd...@orange.com>
Date: 2016-05-31T13:08:39Z
systemvm: pylint conformance work in progress
I kept the changes at the minimum to make pylint happy,
I don't expect regressions.
Using default configuration:
> Your code has been rated at 8.92/10 (previous run: 7.81/10, +1.11)
* pylint 1.5.5,
* astroid 1.4.5
* Python 2.7.11 (default, Feb 22 2016, 09:22:43)
* [GCC 4.8.2]
Previous run is before the patch:
+-----------+--------+-------+----------+------------+
| type | number | % | previous | difference |
+===========+========+=======+==========+============+
| code | 3569 | 64.76 | 3582 | -13.00 |
+-----------+--------+-------+----------+------------+
| docstring | 356 | 6.46 | 330 | +26.00 |
+-----------+--------+-------+----------+------------+
| comment | 773 | 14.03 | 788 | -15.00 |
+-----------+--------+-------+----------+------------+
| empty | 813 | 14.75 | 820 | -7.00 |
+-----------+--------+-------+----------+------------+
+------------+--------+----------+------------+
| type | number | previous | difference |
+============+========+==========+============+
| convention | 261 | 466 | -205 |
+------------+--------+----------+------------+
| refactor | 28 | 48 | -20 |
+------------+--------+----------+------------+
| warning | 43 | 185 | -143 |
+------------+--------+----------+------------+
| error | 4 | 5 | -1 |
+------------+--------+----------+------------+
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1575
This PR is not required in order to stabilize master. Correct?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by romain-dartigues <gi...@git.apache.org>.
Github user romain-dartigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1575
Thank you @wido for pointing it.
Some problems are still present in #1547; should I wait it to be merged to redo the patch on top of it or do you have other suggestions?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1575: systemvm - Python code cleanup
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the issue:
https://github.com/apache/cloudstack/pull/1575
#1547 has been merged, so this no longer does. Maybe you want to send a new PR and close this one?
Sorry for the conflicts though!
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1575: systemvm - Python code cleanup
Posted by romain-dartigues <gi...@git.apache.org>.
Github user romain-dartigues commented on the issue:
https://github.com/apache/cloudstack/pull/1575
As suggested by @wido, rebase + new pull request in #1603.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1575
I think this PR is going to cause a conflict with #1547
That PR fixes a series of issues in the VR which are currently broken, mainly when using multiple ranges inside a POD VLAN with Basic Networking.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1575#discussion_r65183997
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/set_redundant.py ---
@@ -1,47 +0,0 @@
-#!/usr/bin/python
--- End diff --
This is called from `test/systemvm/test_update_config.py`. Not sure what it does and if it works, but at least it is referenced there.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1575: systemvm - Python code cleanup
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the issue:
https://github.com/apache/cloudstack/pull/1575
### ACS CI BVT Run
**Sumarry:**
Build Number 205
Hypervisor xenserver
NetworkType Advanced
Passed=20
Failed=24
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_deploy_vm_with_userdata.py
* test_deployvm_userdata Failed
* test_deployvm_userdata_post Failed
* test_affinity_groups_projects.py
* test_DeployVmAntiAffinityGroup_in_project Failed
* test_vpc_vpn.py
* test_01_redundant_vpc_site2site_vpn Failed
* test_01_vpc_remote_access_vpn Failed
* test_01_vpc_site2site_vpn Failed
* test_scale_vm.py
* ContextSuite context=TestScaleVm>:setup Failing since 5 runs
* test_service_offerings.py
* ContextSuite context=TestServiceOfferings>:setup Failing since 12 runs
* test_routers_iptables_default_policy.py
* test_02_routervm_iptables_policies Failed
* test_01_single_VPC_iptables_policies Failed
* test_routers.py
* ContextSuite context=TestRouterServices>:setup Failing since 2 runs
* test_reset_vm_on_reboot.py
* ContextSuite context=TestResetVmOnReboot>:setup Failing since 4 runs
* test_snapshots.py
* ContextSuite context=TestSnapshotRootDisk>:setup Failing since 3 runs
* test_deploy_vms_with_varied_deploymentplanners.py
* test_deployvm_firstfit Failed
* test_deployvm_userconcentrated Failed
* test_deployvm_userdispersing Failed
* test_list_ids_parameter.py
* ContextSuite context=TestListIdsParams>:setup Failing since 5 runs
* test_multipleips_per_nic.py
* test_nic_secondaryip_add_remove Failed
* test_affinity_groups.py
* test_DeployVmAntiAffinityGroup Failed
* test_network_acl.py
* test_network_acl Failed
* test_volumes.py
* ContextSuite context=TestCreateVolume>:setup Failing since 2 runs
* ContextSuite context=TestVolumes>:setup Failing since 2 runs
* test_nic.py
* test_01_nic Failed
* test_vm_life_cycle.py
* ContextSuite context=TestVMLifeCycle>:setup Failing since 4 runs
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_login.py
test_public_ip_range.py
test_regions.py
test_pvlan.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1575: systemvm - Python code cleanup
Posted by romain-dartigues <gi...@git.apache.org>.
Github user romain-dartigues commented on the issue:
https://github.com/apache/cloudstack/pull/1575
Sorry, I missed your post and I'm not sure @swill question was addressed to me, but if it's the case: this is purely code cleanup and a few bugfixes (only due to typo in code).
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1575: systemvm - Python code cleanup
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the issue:
https://github.com/apache/cloudstack/pull/1575
I think that is correct @swill, this is mainly a code cleanup.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1575
@romain-dartigues in such cases you can fetch the branch of the submitter in your own workspace and build on top of it, push to your own fork on github and open a PR from that. When that gets merged the older one automatically get marked as merged as well. very convenient way of building on each others work.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1575: systemvm - Python code cleanup
Posted by romain-dartigues <gi...@git.apache.org>.
Github user romain-dartigues closed the pull request at:
https://github.com/apache/cloudstack/pull/1575
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by romain-dartigues <gi...@git.apache.org>.
Github user romain-dartigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1575#discussion_r65191566
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/set_redundant.py ---
@@ -1,47 +0,0 @@
-#!/usr/bin/python
--- End diff --
True, I haven't looked for references from the root of the project, my bad...
Unfortunately that mean the [test/systemvm](/apache/cloudstack/tree/8a2391336c305a0209b70062ccdd41bbe0bdff44/test/systemvm) are underused, because the method `set_cl` has been removed a while ago by bdda01d2695ef4426857a85ec435cefcab3a0362.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: systemvm - Python code cleanup
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1575
@romain-dartigues: Work is still underway indeed. I've tested the PR the last few days and it seems to be working.
The problems we saw have been fixed and this code is currently running in production on our systems.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---