Skip to content

Refactor userVmDetailsDao field and remove unusued fields - #2750

Merged
rafaelweingartner merged 2 commits into
apache:masterfrom
shapeblue:removeDuplicatedField
Jul 19, 2018
Merged

Refactor userVmDetailsDao field and remove unusued fields#2750
rafaelweingartner merged 2 commits into
apache:masterfrom
shapeblue:removeDuplicatedField

Conversation

@dhlaluku

Copy link
Copy Markdown
Contributor

Description

This PR refactors a duplicated field vmDetailsDao and _uservmDetailsDao into userVmDetailsDao.
vmDetailsDao, uservmDetailsDao --> userVmDetailsDao

It also removes several unused injected vars as well i.e.;
_domainDao, rulesMgr, _vguTypesDao, _configDepot

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

I have recompiled CloudStack in my local environment and ran the virtual machine related Marvin smoke tests.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.

@borisstoyanov

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@DaanHoogland@rafaelweingartner@resmo@marcaurele I am new to this community and I would like to ask for your assistance with reviewing of this PR.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✖debian. JID-2183

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@dhlaluku a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2184

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@dhlaluku a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@rafaelweingartnerrafaelweingartner self-assigned this Jul 18, 2018

@rafaelweingartnerrafaelweingartner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup! Can you please do a force push to trigger Jenkins again?

Also, can you check if we need all of those protected attributes? Then, if they can be set to private, please do so.

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@rafaelweingartner checked the attributes, it seems like they can be set to private. I will modify them and do some tests before pushing again

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@dhlaluku a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@dhlaluku
dhlalukuforce-pushed the removeDuplicatedField branch from b25de51 to d90042fCompareJuly 18, 2018 17:48
@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2185

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2859)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27365 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2750-t2859-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 62 look OK, 6 have error(s)
Only failed tests results shown below:

TestResultTime (s)Test File
test_provision_certificateError5.21test_certauthority_root.py
ContextSuite context=TestDeployVirtioSCSIVM>:setupError0.00test_deploy_virtio_scsi_vm.py
test_03_vpc_privategw_restart_vpc_cleanupFailure1131.88test_privategw_acl.py
test_01_secure_vm_migrationError3.15test_vm_life_cycle.py
test_02_unsecure_vm_migrationError2.14test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migrationError1.11test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migrationError1.11test_vm_life_cycle.py
test_11_migrate_volume_and_change_offeringError128.34test_volumes.py
test_hostha_enable_ha_when_host_in_maintenanceError3.63test_hostha_kvm.py

@dhlaluku

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@dhlaluku a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@borisstoyanovborisstoyanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think the test failures are present in master as well, will have to address them in a separate PR

@rafaelweingartner
rafaelweingartner merged commit 94dedd6 into apache:masterJul 19, 2018
@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2860)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 25249 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2750-t2860-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 63 look OK, 5 have error(s)
Only failed tests results shown below:

TestResultTime (s)Test File
test_provision_certificateError5.16test_certauthority_root.py
ContextSuite context=TestDeployVirtioSCSIVM>:setupError0.00test_deploy_virtio_scsi_vm.py
test_03_vpc_privategw_restart_vpc_cleanupFailure1121.41test_privategw_acl.py
test_01_secure_vm_migrationError4.14test_vm_life_cycle.py
test_02_unsecure_vm_migrationError1.10test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migrationError1.08test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migrationError1.07test_vm_life_cycle.py
test_11_migrate_volume_and_change_offeringError127.90test_volumes.py

@dhlaluku
dhlaluku deleted the removeDuplicatedField branch July 19, 2018 13:08
borisstoyanov pushed a commit to shapeblue/cloudstack that referenced this pull request Jul 23, 2018
* Refactor userVmDetailsDao field and remove unusued fields
* Setting injected attributes to private instead of protected and amending unit test
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@dhlaluku@borisstoyanov@blueorangutan@marcaurele@rafaelweingartner